From d8c7d47702023f820435a0944b16220d8fa7e70e Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Sat, 10 Feb 2024 15:43:18 -0300 Subject: [PATCH 01/10] feat: optional xblocks --- cms/djangoapps/contentstore/views/block.py | 1 + .../models/settings/course_metadata.py | 1 + .../js/views/modals/course_outline_modals.js | 46 +++++++++++++++++-- cms/static/sass/elements/_modal-window.scss | 8 +++- cms/templates/course_outline.html | 2 +- cms/templates/js/course-outline.underscore | 3 ++ .../js/optional-content-editor.underscore | 5 ++ .../course_api/blocks/serializers.py | 1 + .../blocks/transformers/block_completion.py | 2 +- .../course_home_api/outline/serializers.py | 1 + .../outline/tests/test_view.py | 15 ++++++ .../course_home_api/progress/serializers.py | 1 + .../progress/tests/test_views.py | 10 ++++ .../course_home_api/progress/views.py | 1 + lms/djangoapps/courseware/courses.py | 10 +++- openedx/features/course_experience/utils.py | 1 + xmodule/modulestore/inheritance.py | 11 +++++ 17 files changed, 111 insertions(+), 8 deletions(-) create mode 100644 cms/templates/js/optional-content-editor.underscore diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index 7305bc9adc89..8b9680c2a3a3 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -1321,6 +1321,7 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F 'group_access': xblock.group_access, 'user_partitions': user_partitions, 'show_correctness': xblock.show_correctness, + 'optional_content': xblock.optional_content, }) if xblock.category == 'sequential': diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index c072a4ae552b..4be641f82546 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -78,6 +78,7 @@ class CourseMetadata: 'highlights_enabled_for_messaging', 'is_onboarding_exam', 'discussions_settings', + 'optional', ] @classmethod diff --git a/cms/static/js/views/modals/course_outline_modals.js b/cms/static/js/views/modals/course_outline_modals.js index a10a4f23749d..1197af63e65a 100644 --- a/cms/static/js/views/modals/course_outline_modals.js +++ b/cms/static/js/views/modals/course_outline_modals.js @@ -18,7 +18,7 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', ReleaseDateEditor, DueDateEditor, SelfPacedDueDateEditor, GradingEditor, PublishEditor, AbstractVisibilityEditor, StaffLockEditor, UnitAccessEditor, ContentVisibilityEditor, TimedExaminationPreferenceEditor, AccessEditor, ShowCorrectnessEditor, HighlightsEditor, HighlightsEnableXBlockModal, HighlightsEnableEditor, - DiscussionEditor; + DiscussionEditor, OptionalContentEditor; CourseOutlineXBlockModal = BaseModal.extend({ events: _.extend({}, BaseModal.prototype.events, { @@ -1201,6 +1201,46 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', } }); + OptionalContentEditor = AbstractEditor.extend( + { + templateName: 'optional-content-editor', + className: 'edit-optional-content', + + afterRender: function() { + AbstractEditor.prototype.afterRender.call(this); + this.setValue(this.model.get("optional_content")); + }, + + setValue: function(value) { + this.$('input[name=optional_content]').prop('checked', value); + }, + + currentValue: function() { + return this.$('input[name=optional_content]').is(':checked'); + }, + + hasChanges: function() { + return this.model.get('optional_content') !== this.currentValue(); + }, + + getRequestData: function() { + if (this.hasChanges()) { + return { + publish: 'republish', + metadata: { + optional_content: this.currentValue() + } + }; + } else { + return {}; + } + }, + getContext: function() { + return { + optional_content: this.model.get('optional_content') + }; + }, + }) return { getModal: function(type, xblockInfo, options) { if (type === 'edit') { @@ -1240,10 +1280,10 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', } ]; if (xblockInfo.isChapter()) { - tabs[0].editors = [ReleaseDateEditor]; + tabs[0].editors = [ReleaseDateEditor, OptionalContentEditor]; tabs[1].editors = [StaffLockEditor]; } else if (xblockInfo.isSequential()) { - tabs[0].editors = [ReleaseDateEditor, GradingEditor, DueDateEditor]; + tabs[0].editors = [ReleaseDateEditor, GradingEditor, DueDateEditor, OptionalContentEditor]; tabs[1].editors = [ContentVisibilityEditor, ShowCorrectnessEditor]; if (course.get('self_paced') && course.get('is_custom_relative_dates_active')) { tabs[0].editors.push(SelfPacedDueDateEditor); diff --git a/cms/static/sass/elements/_modal-window.scss b/cms/static/sass/elements/_modal-window.scss index b6508ef49a01..4a91aced1316 100644 --- a/cms/static/sass/elements/_modal-window.scss +++ b/cms/static/sass/elements/_modal-window.scss @@ -747,6 +747,7 @@ .edit-discussion, .edit-staff-lock, + .edit-optional-content, .edit-content-visibility, .edit-unit-access { margin-bottom: $baseline; @@ -760,19 +761,20 @@ // UI: staff lock and discussion .edit-discussion, .edit-staff-lock, + .edit-optional-content, .edit-settings-timed-examination, .edit-unit-access { .checkbox-cosmetic .input-checkbox { @extend %cont-text-sr; // CASE: unchecked - ~ .tip-warning { + ~.tip-warning { display: block; } // CASE: checked &:checked { - ~ .tip-warning { + ~.tip-warning { display: none; } } @@ -832,6 +834,7 @@ .edit-discussion, .edit-unit-access, + .edit-optional-content, .edit-staff-lock { .modal-section-content { @include font-size(16); @@ -874,6 +877,7 @@ .edit-discussion, .edit-unit-access, + .edit-optional-content, .edit-staff-lock { .modal-section-content { @include font-size(16); diff --git a/cms/templates/course_outline.html b/cms/templates/course_outline.html index f9f499c87bd8..f24a574be9fe 100644 --- a/cms/templates/course_outline.html +++ b/cms/templates/course_outline.html @@ -29,7 +29,7 @@ <%block name="header_extras"> -% for template_name in ['course-outline', 'xblock-string-field-editor', 'basic-modal', 'modal-button', 'course-outline-modal', 'due-date-editor', 'self-paced-due-date-editor', 'release-date-editor', 'grading-editor', 'publish-editor', 'staff-lock-editor', 'unit-access-editor', 'discussion-editor', 'content-visibility-editor', 'verification-access-editor', 'timed-examination-preference-editor', 'access-editor', 'settings-modal-tabs', 'show-correctness-editor', 'highlights-editor', 'highlights-enable-editor', 'course-highlights-enable']: +% for template_name in ['course-outline', 'xblock-string-field-editor', 'basic-modal', 'modal-button', 'course-outline-modal', 'due-date-editor', 'self-paced-due-date-editor', 'release-date-editor', 'grading-editor', 'publish-editor', 'staff-lock-editor', 'unit-access-editor', 'discussion-editor', 'content-visibility-editor', 'verification-access-editor', 'timed-examination-preference-editor', 'access-editor', 'settings-modal-tabs', 'show-correctness-editor', 'highlights-editor', 'highlights-enable-editor', 'course-highlights-enable', 'optional-content-editor']: diff --git a/cms/templates/js/course-outline.underscore b/cms/templates/js/course-outline.underscore index 7f82410196bb..636d37bd131a 100644 --- a/cms/templates/js/course-outline.underscore +++ b/cms/templates/js/course-outline.underscore @@ -225,6 +225,9 @@ if (is_proctored_exam) { <% if (xblockInfo.get('release_date')) { %> <%- xblockInfo.get('release_date') %> <% } %> + <% if (xblockInfo.get('optional_content')) { %> + - <%- gettext('Optional') %> + <% } %> <% } %>

diff --git a/cms/templates/js/optional-content-editor.underscore b/cms/templates/js/optional-content-editor.underscore new file mode 100644 index 000000000000..8b6101488cc3 --- /dev/null +++ b/cms/templates/js/optional-content-editor.underscore @@ -0,0 +1,5 @@ + diff --git a/lms/djangoapps/course_api/blocks/serializers.py b/lms/djangoapps/course_api/blocks/serializers.py index ac031600a02c..68c4ab0f1840 100644 --- a/lms/djangoapps/course_api/blocks/serializers.py +++ b/lms/djangoapps/course_api/blocks/serializers.py @@ -56,6 +56,7 @@ def __init__( SupportedFieldType('has_score'), SupportedFieldType('has_scheduled_content'), SupportedFieldType('weight'), + SupportedFieldType('optional_content'), SupportedFieldType('show_correctness'), # 'student_view_data' SupportedFieldType(StudentViewTransformer.STUDENT_VIEW_DATA, StudentViewTransformer), diff --git a/lms/djangoapps/course_api/blocks/transformers/block_completion.py b/lms/djangoapps/course_api/blocks/transformers/block_completion.py index 472555c4c7f9..3825137e1abc 100644 --- a/lms/djangoapps/course_api/blocks/transformers/block_completion.py +++ b/lms/djangoapps/course_api/blocks/transformers/block_completion.py @@ -43,7 +43,7 @@ def get_block_completion(cls, block_structure, block_key): @classmethod def collect(cls, block_structure): - block_structure.request_xblock_fields('completion_mode') + block_structure.request_xblock_fields('completion_mode', 'optional_content') @staticmethod def _is_block_excluded(block_structure, block_key): diff --git a/lms/djangoapps/course_home_api/outline/serializers.py b/lms/djangoapps/course_home_api/outline/serializers.py index 20b205e3040d..d337130fda18 100644 --- a/lms/djangoapps/course_home_api/outline/serializers.py +++ b/lms/djangoapps/course_home_api/outline/serializers.py @@ -54,6 +54,7 @@ def get_blocks(self, block): # pylint: disable=missing-function-docstring 'resume_block': block.get('resume_block', False), 'type': block_type, 'has_scheduled_content': block.get('has_scheduled_content'), + 'optional_content': block.get('optional_content'), }, } for child in children: diff --git a/lms/djangoapps/course_home_api/outline/tests/test_view.py b/lms/djangoapps/course_home_api/outline/tests/test_view.py index f02ad646ef48..79a037118b45 100644 --- a/lms/djangoapps/course_home_api/outline/tests/test_view.py +++ b/lms/djangoapps/course_home_api/outline/tests/test_view.py @@ -437,3 +437,18 @@ def test_cannot_enroll_if_full(self): self.update_course_and_overview() CourseEnrollment.enroll(UserFactory(), self.course.id) # grr, some rando took our spot! self.assert_can_enroll(False) + + def test_optional_content(self): + CourseEnrollment.enroll(self.user, self.course.id) + assert not self.course.optional_content + response = self.client.get(self.url) + for block in response.data['course_blocks']['blocks'].values(): + assert not block['optional_content'] + + def test_optional_content_true(self): + self.course.optional_content = True + self.update_course_and_overview() + CourseEnrollment.enroll(self.user, self.course.id) + response = self.client.get(self.url) + for block in response.data['course_blocks']['blocks'].values(): + assert block['optional_content'] diff --git a/lms/djangoapps/course_home_api/progress/serializers.py b/lms/djangoapps/course_home_api/progress/serializers.py index 190cd76bf9b5..98667314674b 100644 --- a/lms/djangoapps/course_home_api/progress/serializers.py +++ b/lms/djangoapps/course_home_api/progress/serializers.py @@ -134,6 +134,7 @@ class ProgressTabSerializer(VerifiedModeSerializer): access_expiration = serializers.DictField() certificate_data = CertificateDataSerializer() completion_summary = serializers.DictField() + optional_completion_summary = serializers.DictField() course_grade = CourseGradeSerializer() credit_course_requirements = serializers.DictField() end = serializers.DateTimeField() diff --git a/lms/djangoapps/course_home_api/progress/tests/test_views.py b/lms/djangoapps/course_home_api/progress/tests/test_views.py index d13ebec29c21..253d9a544e97 100644 --- a/lms/djangoapps/course_home_api/progress/tests/test_views.py +++ b/lms/djangoapps/course_home_api/progress/tests/test_views.py @@ -314,3 +314,13 @@ def test_course_grade_considers_subsection_grade_visibility(self, is_staff, expe assert response.status_code == 200 assert response.data['course_grade']['percent'] == expected_percent assert response.data['course_grade']['is_passing'] == (expected_percent >= 0.5) + + def test_optional_content(self): + CourseEnrollment.enroll(self.user, self.course.id) + response = self.client.get(self.url) + assert response.status_code == 200 + assert response.data['optional_completion_summary'] == { + 'complete_count': 0, + 'incomplete_count': 0, + 'locked_count': 0, + } diff --git a/lms/djangoapps/course_home_api/progress/views.py b/lms/djangoapps/course_home_api/progress/views.py index dc0ea63525f7..ad1bbd4ef5ce 100644 --- a/lms/djangoapps/course_home_api/progress/views.py +++ b/lms/djangoapps/course_home_api/progress/views.py @@ -246,6 +246,7 @@ def get(self, request, *args, **kwargs): 'access_expiration': access_expiration, 'certificate_data': get_cert_data(student, course, enrollment_mode, course_grade), 'completion_summary': get_course_blocks_completion_summary(course_key, student), + 'optional_completion_summary': get_course_blocks_completion_summary(course_key, student, optional=True), 'course_grade': course_grade, 'credit_course_requirements': credit_course_requirements(course_key, student), 'end': course.end, diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 6ded860329fb..7c992bc7cfe2 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -552,7 +552,7 @@ def get_course_assignment_date_blocks(course, user, request, num_return=None, @request_cached() -def get_course_blocks_completion_summary(course_key, user): +def get_course_blocks_completion_summary(course_key, user, optional=False): """ Returns an object with the number of complete units, incomplete units, and units that contain gated content for the given course. The complete and incomplete counts only reflect units that are able to be completed by @@ -566,10 +566,18 @@ def get_course_blocks_completion_summary(course_key, user): course_usage_key = store.make_course_usage_key(course_key) block_data = get_course_blocks(user, course_usage_key, allow_start_dates_in_future=True, include_completion=True) + def _is_optional(*keys): + for key in keys: + if block_data.get_xblock_field(key, 'optional_content', False): + return True + return False + complete_count, incomplete_count, locked_count = 0, 0, 0 for section_key in block_data.get_children(course_usage_key): # pylint: disable=too-many-nested-blocks for subsection_key in block_data.get_children(section_key): for unit_key in block_data.get_children(subsection_key): + if optional != _is_optional(section_key, subsection_key, unit_key): + continue complete = block_data.get_xblock_field(unit_key, 'complete', False) contains_gated_content = block_data.get_xblock_field(unit_key, 'contains_gated_content', False) if contains_gated_content: diff --git a/openedx/features/course_experience/utils.py b/openedx/features/course_experience/utils.py index d58b54f6139f..092f6b41405e 100644 --- a/openedx/features/course_experience/utils.py +++ b/openedx/features/course_experience/utils.py @@ -115,6 +115,7 @@ def recurse_mark_auth_denial(block): 'completion', 'complete', 'resume_block', + 'optional_content', ], allow_start_dates_in_future=allow_start_dates_in_future, ) diff --git a/xmodule/modulestore/inheritance.py b/xmodule/modulestore/inheritance.py index e75b9ab083c4..46948870266d 100644 --- a/xmodule/modulestore/inheritance.py +++ b/xmodule/modulestore/inheritance.py @@ -238,6 +238,17 @@ class InheritanceMixin(XBlockMixin): scope=Scope.settings ) + optional_content = Boolean( + display_name=_('Optional'), + help=_( + 'Set this to true to mark this block as optional.' + 'Progress in this block won\'t count towards course completion progress' + 'and will count as optional progress instead.' + ), + default=False, + scope=Scope.settings, + ) + @property def close_date(self): """ From 1a4994b6107cbb3cb8931f1fe70869be8c2e7839 Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Thu, 29 Feb 2024 03:15:24 -0300 Subject: [PATCH 02/10] feat: address PR comments --- .../contentstore/tests/test_utils.py | 54 +++++++++++++++++++ cms/djangoapps/contentstore/utils.py | 14 +++++ cms/djangoapps/contentstore/views/block.py | 8 ++- .../models/settings/course_metadata.py | 2 +- cms/static/js/models/xblock_info.js | 4 ++ .../js/views/modals/course_outline_modals.js | 28 +++++----- cms/static/sass/elements/_modal-window.scss | 20 ++++--- cms/templates/course_outline.html | 2 +- cms/templates/js/course-outline.underscore | 2 +- .../js/optional-completion-editor.underscore | 26 +++++++++ .../js/optional-content-editor.underscore | 5 -- .../course_api/blocks/serializers.py | 2 +- .../blocks/transformers/block_completion.py | 4 +- .../course_home_api/outline/serializers.py | 2 +- .../outline/tests/test_view.py | 12 ++--- .../progress/tests/test_views.py | 2 +- lms/djangoapps/courseware/courses.py | 2 +- openedx/features/course_experience/utils.py | 2 +- xmodule/modulestore/inheritance.py | 2 +- 19 files changed, 151 insertions(+), 42 deletions(-) create mode 100644 cms/templates/js/optional-completion-editor.underscore delete mode 100644 cms/templates/js/optional-content-editor.underscore diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 5b4f4338b2b6..8c39ed7a5ff5 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -336,6 +336,60 @@ def test_no_inheritance_for_orphan(self): self.assertFalse(utils.ancestor_has_staff_lock(self.orphan)) +class InheritedOptionalCompletionTest(CourseTestCase): + """Tests for determining if an xblock inherits optional completion.""" + + def setUp(self): + super().setUp() + chapter = BlockFactory.create(category='chapter', parent=self.course) + sequential = BlockFactory.create(category='sequential', parent=chapter) + vertical = BlockFactory.create(category='vertical', parent=sequential) + html = BlockFactory.create(category='html', parent=vertical) + problem = BlockFactory.create( + category='problem', parent=vertical, data="" + ) + self.chapter = self.store.get_item(chapter.location) + self.sequential = self.store.get_item(sequential.location) + self.vertical = self.store.get_item(vertical.location) + self.html = self.store.get_item(html.location) + self.problem = self.store.get_item(problem.location) + + def set_optional_completion(self, xblock, value): + """ Sets optional_completion to specified value and calls update_item to persist the change. """ + xblock.optional_completion = value + self.store.update_item(xblock, self.user.id) + + def update_optional_completions(self, chapter, sequential, vertical): + self.set_optional_completion(self.chapter, chapter) + self.set_optional_completion(self.sequential, sequential) + self.set_optional_completion(self.vertical, vertical) + + def test_no_inheritance(self): + """Tests that vertical with no optional ancestors does not have an inherited optional completion""" + self.update_optional_completions(False, False, False) + self.assertFalse(utils.ancestor_has_optional_completion(self.vertical)) + self.update_optional_completions(False, False, True) + self.assertFalse(utils.ancestor_has_optional_completion(self.vertical)) + + def test_inheritance_in_optional_section(self): + """Tests that a vertical in an optional section has an inherited optional completion""" + self.update_optional_completions(True, False, False) + self.assertTrue(utils.ancestor_has_optional_completion(self.vertical)) + self.update_optional_completions(True, False, True) + self.assertTrue(utils.ancestor_has_optional_completion(self.vertical)) + + def test_inheritance_in_optional_subsection(self): + """Tests that a vertical in an optional subsection has an inherited optional completion""" + self.update_optional_completions(False, True, False) + self.assertTrue(utils.ancestor_has_optional_completion(self.vertical)) + self.update_optional_completions(False, True, True) + self.assertTrue(utils.ancestor_has_optional_completion(self.vertical)) + + def test_no_inheritance_for_orphan(self): + """Tests that an orphaned xblock does not inherit optional completion""" + self.assertFalse(utils.ancestor_has_optional_completion(self.orphan)) + + class GroupVisibilityTest(CourseTestCase): """ Test content group access rules. diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 94f689d2cb02..a92d479d4fd0 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -354,6 +354,20 @@ def ancestor_has_staff_lock(xblock, parent_xblock=None): return parent_xblock.visible_to_staff_only +def ancestor_has_optional_completion(xblock, parent_xblock=None): + """ + Returns True iff one of xblock's ancestors has optional completion. + Can avoid mongo query by passing in parent_xblock. + """ + if parent_xblock is None: + parent_location = modulestore().get_parent_location(xblock.location, + revision=ModuleStoreEnum.RevisionOption.draft_preferred) + if not parent_location: + return False + parent_xblock = modulestore().get_item(parent_location) + return parent_xblock.optional_completion + + def reverse_url(handler_name, key_name=None, key_value=None, kwargs=None): """ Creates the URL for the given handler. diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index 8b9680c2a3a3..92d3397a3955 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -60,6 +60,7 @@ from xmodule.x_module import AUTHOR_VIEW, PREVIEW_VIEWS, STUDENT_VIEW, STUDIO_VIEW # lint-amnesty, pylint: disable=wrong-import-order from ..utils import ( + ancestor_has_optional_completion, ancestor_has_staff_lock, find_release_date_source, find_staff_lock_source, @@ -1321,7 +1322,7 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F 'group_access': xblock.group_access, 'user_partitions': user_partitions, 'show_correctness': xblock.show_correctness, - 'optional_content': xblock.optional_content, + 'optional_completion': xblock.optional_completion, }) if xblock.category == 'sequential': @@ -1406,6 +1407,11 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F xblock_info['ancestor_has_staff_lock'] = False if course_outline: + if xblock_info['optional_completion']: + xblock_info['ancestor_has_optional_completion'] = ancestor_has_optional_completion(xblock, parent_xblock) + else: + xblock_info['ancestor_has_optional_completion'] = False + if xblock_info['has_explicit_staff_lock']: xblock_info['staff_only_message'] = True elif child_info and child_info['children']: diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 4be641f82546..3fde376bb781 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -78,7 +78,7 @@ class CourseMetadata: 'highlights_enabled_for_messaging', 'is_onboarding_exam', 'discussions_settings', - 'optional', + 'optional_completion', ] @classmethod diff --git a/cms/static/js/models/xblock_info.js b/cms/static/js/models/xblock_info.js index 983edc4e5648..7c46a3e201c9 100644 --- a/cms/static/js/models/xblock_info.js +++ b/cms/static/js/models/xblock_info.js @@ -120,6 +120,10 @@ define( */ ancestor_has_staff_lock: null, /** + * True if this any of this xblock's ancestors are optional for completion. + */ + ancestor_has_optional_completion: null, + /** * The xblock which is determining the staff lock value. For instance, for a unit, * this will either be the parent subsection or the grandparent section. * This can be null if the xblock has no inherited staff lock. Will only be present if diff --git a/cms/static/js/views/modals/course_outline_modals.js b/cms/static/js/views/modals/course_outline_modals.js index 1197af63e65a..098e55d51079 100644 --- a/cms/static/js/views/modals/course_outline_modals.js +++ b/cms/static/js/views/modals/course_outline_modals.js @@ -18,7 +18,7 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', ReleaseDateEditor, DueDateEditor, SelfPacedDueDateEditor, GradingEditor, PublishEditor, AbstractVisibilityEditor, StaffLockEditor, UnitAccessEditor, ContentVisibilityEditor, TimedExaminationPreferenceEditor, AccessEditor, ShowCorrectnessEditor, HighlightsEditor, HighlightsEnableXBlockModal, HighlightsEnableEditor, - DiscussionEditor, OptionalContentEditor; + DiscussionEditor, OptionalCompletionEditor; CourseOutlineXBlockModal = BaseModal.extend({ events: _.extend({}, BaseModal.prototype.events, { @@ -1201,26 +1201,26 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', } }); - OptionalContentEditor = AbstractEditor.extend( + OptionalCompletionEditor = AbstractEditor.extend( { - templateName: 'optional-content-editor', - className: 'edit-optional-content', + templateName: 'optional-completion-editor', + className: 'edit-optional-completion', afterRender: function() { AbstractEditor.prototype.afterRender.call(this); - this.setValue(this.model.get("optional_content")); + this.setValue(this.model.get("optional_completion")); }, setValue: function(value) { - this.$('input[name=optional_content]').prop('checked', value); + this.$('input[name=optional_completion]').prop('checked', value); }, currentValue: function() { - return this.$('input[name=optional_content]').is(':checked'); + return this.$('input[name=optional_completion]').is(':checked'); }, hasChanges: function() { - return this.model.get('optional_content') !== this.currentValue(); + return this.model.get('optional_completion') !== this.currentValue(); }, getRequestData: function() { @@ -1228,16 +1228,18 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', return { publish: 'republish', metadata: { - optional_content: this.currentValue() + optional_completion: this.currentValue() } }; } else { return {}; } }, + getContext: function() { return { - optional_content: this.model.get('optional_content') + optional_completion: this.model.get('optional_completion'), + optionalAncestor: this.model.get('ancestor_has_optional_completion') }; }, }) @@ -1265,7 +1267,7 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', editors: [] }; if (xblockInfo.isVertical()) { - editors = [StaffLockEditor, UnitAccessEditor, DiscussionEditor]; + editors = [StaffLockEditor, UnitAccessEditor, DiscussionEditor, OptionalCompletionEditor]; } else { tabs = [ { @@ -1280,10 +1282,10 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', } ]; if (xblockInfo.isChapter()) { - tabs[0].editors = [ReleaseDateEditor, OptionalContentEditor]; + tabs[0].editors = [ReleaseDateEditor, OptionalCompletionEditor]; tabs[1].editors = [StaffLockEditor]; } else if (xblockInfo.isSequential()) { - tabs[0].editors = [ReleaseDateEditor, GradingEditor, DueDateEditor, OptionalContentEditor]; + tabs[0].editors = [ReleaseDateEditor, GradingEditor, DueDateEditor, OptionalCompletionEditor]; tabs[1].editors = [ContentVisibilityEditor, ShowCorrectnessEditor]; if (course.get('self_paced') && course.get('is_custom_relative_dates_active')) { tabs[0].editors.push(SelfPacedDueDateEditor); diff --git a/cms/static/sass/elements/_modal-window.scss b/cms/static/sass/elements/_modal-window.scss index 4a91aced1316..92ccfe2f1876 100644 --- a/cms/static/sass/elements/_modal-window.scss +++ b/cms/static/sass/elements/_modal-window.scss @@ -747,7 +747,7 @@ .edit-discussion, .edit-staff-lock, - .edit-optional-content, + .edit-optional-completion, .edit-content-visibility, .edit-unit-access { margin-bottom: $baseline; @@ -761,20 +761,20 @@ // UI: staff lock and discussion .edit-discussion, .edit-staff-lock, - .edit-optional-content, + .edit-optional-completion, .edit-settings-timed-examination, .edit-unit-access { .checkbox-cosmetic .input-checkbox { @extend %cont-text-sr; // CASE: unchecked - ~.tip-warning { + ~ .tip-warning { display: block; } // CASE: checked &:checked { - ~.tip-warning { + ~ .tip-warning { display: none; } } @@ -832,9 +832,17 @@ } } + .edit-optional-completion { + .field-message { + @extend %t-copy-sub1; + color: $gray-d1; + margin-bottom: ($baseline/4); + } + } + .edit-discussion, .edit-unit-access, - .edit-optional-content, + .edit-optional-completion, .edit-staff-lock { .modal-section-content { @include font-size(16); @@ -877,7 +885,7 @@ .edit-discussion, .edit-unit-access, - .edit-optional-content, + .edit-optional-completion, .edit-staff-lock { .modal-section-content { @include font-size(16); diff --git a/cms/templates/course_outline.html b/cms/templates/course_outline.html index f24a574be9fe..e61999553bd5 100644 --- a/cms/templates/course_outline.html +++ b/cms/templates/course_outline.html @@ -29,7 +29,7 @@ <%block name="header_extras"> -% for template_name in ['course-outline', 'xblock-string-field-editor', 'basic-modal', 'modal-button', 'course-outline-modal', 'due-date-editor', 'self-paced-due-date-editor', 'release-date-editor', 'grading-editor', 'publish-editor', 'staff-lock-editor', 'unit-access-editor', 'discussion-editor', 'content-visibility-editor', 'verification-access-editor', 'timed-examination-preference-editor', 'access-editor', 'settings-modal-tabs', 'show-correctness-editor', 'highlights-editor', 'highlights-enable-editor', 'course-highlights-enable', 'optional-content-editor']: +% for template_name in ['course-outline', 'xblock-string-field-editor', 'basic-modal', 'modal-button', 'course-outline-modal', 'due-date-editor', 'self-paced-due-date-editor', 'release-date-editor', 'grading-editor', 'publish-editor', 'staff-lock-editor', 'unit-access-editor', 'discussion-editor', 'content-visibility-editor', 'verification-access-editor', 'timed-examination-preference-editor', 'access-editor', 'settings-modal-tabs', 'show-correctness-editor', 'highlights-editor', 'highlights-enable-editor', 'course-highlights-enable', 'optional-completion-editor']: diff --git a/cms/templates/js/course-outline.underscore b/cms/templates/js/course-outline.underscore index 636d37bd131a..6cecb3d7a584 100644 --- a/cms/templates/js/course-outline.underscore +++ b/cms/templates/js/course-outline.underscore @@ -225,7 +225,7 @@ if (is_proctored_exam) { <% if (xblockInfo.get('release_date')) { %> <%- xblockInfo.get('release_date') %> <% } %> - <% if (xblockInfo.get('optional_content')) { %> + <% if (xblockInfo.get('optional_completion')) { %> - <%- gettext('Optional') %> <% } %> <% } %> diff --git a/cms/templates/js/optional-completion-editor.underscore b/cms/templates/js/optional-completion-editor.underscore new file mode 100644 index 000000000000..e6ae730124ff --- /dev/null +++ b/cms/templates/js/optional-completion-editor.underscore @@ -0,0 +1,26 @@ +
+ + +
diff --git a/cms/templates/js/optional-content-editor.underscore b/cms/templates/js/optional-content-editor.underscore deleted file mode 100644 index 8b6101488cc3..000000000000 --- a/cms/templates/js/optional-content-editor.underscore +++ /dev/null @@ -1,5 +0,0 @@ - diff --git a/lms/djangoapps/course_api/blocks/serializers.py b/lms/djangoapps/course_api/blocks/serializers.py index 68c4ab0f1840..1fe137fad9fc 100644 --- a/lms/djangoapps/course_api/blocks/serializers.py +++ b/lms/djangoapps/course_api/blocks/serializers.py @@ -56,7 +56,7 @@ def __init__( SupportedFieldType('has_score'), SupportedFieldType('has_scheduled_content'), SupportedFieldType('weight'), - SupportedFieldType('optional_content'), + SupportedFieldType('optional_completion'), SupportedFieldType('show_correctness'), # 'student_view_data' SupportedFieldType(StudentViewTransformer.STUDENT_VIEW_DATA, StudentViewTransformer), diff --git a/lms/djangoapps/course_api/blocks/transformers/block_completion.py b/lms/djangoapps/course_api/blocks/transformers/block_completion.py index 3825137e1abc..85eeef1e2162 100644 --- a/lms/djangoapps/course_api/blocks/transformers/block_completion.py +++ b/lms/djangoapps/course_api/blocks/transformers/block_completion.py @@ -14,7 +14,7 @@ class BlockCompletionTransformer(BlockStructureTransformer): Keep track of the completion of each block within the block structure. """ READ_VERSION = 1 - WRITE_VERSION = 1 + WRITE_VERSION = 2 COMPLETION = 'completion' COMPLETE = 'complete' RESUME_BLOCK = 'resume_block' @@ -43,7 +43,7 @@ def get_block_completion(cls, block_structure, block_key): @classmethod def collect(cls, block_structure): - block_structure.request_xblock_fields('completion_mode', 'optional_content') + block_structure.request_xblock_fields('completion_mode', 'optional_completion') @staticmethod def _is_block_excluded(block_structure, block_key): diff --git a/lms/djangoapps/course_home_api/outline/serializers.py b/lms/djangoapps/course_home_api/outline/serializers.py index d337130fda18..82739aeb0dcf 100644 --- a/lms/djangoapps/course_home_api/outline/serializers.py +++ b/lms/djangoapps/course_home_api/outline/serializers.py @@ -54,7 +54,7 @@ def get_blocks(self, block): # pylint: disable=missing-function-docstring 'resume_block': block.get('resume_block', False), 'type': block_type, 'has_scheduled_content': block.get('has_scheduled_content'), - 'optional_content': block.get('optional_content'), + 'optional_completion': block.get('optional_completion', False), }, } for child in children: diff --git a/lms/djangoapps/course_home_api/outline/tests/test_view.py b/lms/djangoapps/course_home_api/outline/tests/test_view.py index 79a037118b45..ba4c0887096e 100644 --- a/lms/djangoapps/course_home_api/outline/tests/test_view.py +++ b/lms/djangoapps/course_home_api/outline/tests/test_view.py @@ -438,17 +438,17 @@ def test_cannot_enroll_if_full(self): CourseEnrollment.enroll(UserFactory(), self.course.id) # grr, some rando took our spot! self.assert_can_enroll(False) - def test_optional_content(self): + def test_optional_completion(self): CourseEnrollment.enroll(self.user, self.course.id) - assert not self.course.optional_content + assert not self.course.optional_completion response = self.client.get(self.url) for block in response.data['course_blocks']['blocks'].values(): - assert not block['optional_content'] + assert not block['optional_completion'] - def test_optional_content_true(self): - self.course.optional_content = True + def test_optional_completion_true(self): + self.course.optional_completion = True self.update_course_and_overview() CourseEnrollment.enroll(self.user, self.course.id) response = self.client.get(self.url) for block in response.data['course_blocks']['blocks'].values(): - assert block['optional_content'] + assert block['optional_completion'] diff --git a/lms/djangoapps/course_home_api/progress/tests/test_views.py b/lms/djangoapps/course_home_api/progress/tests/test_views.py index 253d9a544e97..60c648a5e94b 100644 --- a/lms/djangoapps/course_home_api/progress/tests/test_views.py +++ b/lms/djangoapps/course_home_api/progress/tests/test_views.py @@ -315,7 +315,7 @@ def test_course_grade_considers_subsection_grade_visibility(self, is_staff, expe assert response.data['course_grade']['percent'] == expected_percent assert response.data['course_grade']['is_passing'] == (expected_percent >= 0.5) - def test_optional_content(self): + def test_optional_completion(self): CourseEnrollment.enroll(self.user, self.course.id) response = self.client.get(self.url) assert response.status_code == 200 diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 7c992bc7cfe2..4963a5cc5db3 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -568,7 +568,7 @@ def get_course_blocks_completion_summary(course_key, user, optional=False): def _is_optional(*keys): for key in keys: - if block_data.get_xblock_field(key, 'optional_content', False): + if block_data.get_xblock_field(key, 'optional_completion', False): return True return False diff --git a/openedx/features/course_experience/utils.py b/openedx/features/course_experience/utils.py index 092f6b41405e..8dc2f5d248be 100644 --- a/openedx/features/course_experience/utils.py +++ b/openedx/features/course_experience/utils.py @@ -115,7 +115,7 @@ def recurse_mark_auth_denial(block): 'completion', 'complete', 'resume_block', - 'optional_content', + 'optional_completion', ], allow_start_dates_in_future=allow_start_dates_in_future, ) diff --git a/xmodule/modulestore/inheritance.py b/xmodule/modulestore/inheritance.py index 46948870266d..21a30861892d 100644 --- a/xmodule/modulestore/inheritance.py +++ b/xmodule/modulestore/inheritance.py @@ -238,7 +238,7 @@ class InheritanceMixin(XBlockMixin): scope=Scope.settings ) - optional_content = Boolean( + optional_completion = Boolean( display_name=_('Optional'), help=_( 'Set this to true to mark this block as optional.' From 96750c6d96e06e0dc31600bb91b01bb2d7d0c215 Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Sat, 9 Mar 2024 19:57:55 -0300 Subject: [PATCH 03/10] fix: use completion tracking flag and small docstring changes --- cms/djangoapps/contentstore/utils.py | 4 ++-- cms/djangoapps/contentstore/views/block.py | 5 +---- cms/static/js/views/modals/course_outline_modals.js | 12 ++++++++---- cms/templates/base.html | 2 ++ .../js/optional-completion-editor.underscore | 4 ++-- .../blocks/transformers/block_completion.py | 2 +- .../course_home_api/outline/tests/test_view.py | 4 ++-- lms/djangoapps/courseware/courses.py | 8 +++++++- 8 files changed, 25 insertions(+), 16 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index a92d479d4fd0..9045eda25806 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -342,7 +342,7 @@ def find_staff_lock_source(xblock): def ancestor_has_staff_lock(xblock, parent_xblock=None): """ - Returns True iff one of xblock's ancestors has staff lock. + Returns True if one of xblock's ancestors has staff lock. Can avoid mongo query by passing in parent_xblock. """ if parent_xblock is None: @@ -356,7 +356,7 @@ def ancestor_has_staff_lock(xblock, parent_xblock=None): def ancestor_has_optional_completion(xblock, parent_xblock=None): """ - Returns True iff one of xblock's ancestors has optional completion. + Returns True if one of xblock's ancestors has optional completion. Can avoid mongo query by passing in parent_xblock. """ if parent_xblock is None: diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index 92d3397a3955..55dbffa124ca 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -1407,10 +1407,7 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F xblock_info['ancestor_has_staff_lock'] = False if course_outline: - if xblock_info['optional_completion']: - xblock_info['ancestor_has_optional_completion'] = ancestor_has_optional_completion(xblock, parent_xblock) - else: - xblock_info['ancestor_has_optional_completion'] = False + xblock_info['ancestor_has_optional_completion'] = ancestor_has_optional_completion(xblock, parent_xblock) if xblock_info['has_explicit_staff_lock']: xblock_info['staff_only_message'] = True diff --git a/cms/static/js/views/modals/course_outline_modals.js b/cms/static/js/views/modals/course_outline_modals.js index 098e55d51079..fdfa38b62221 100644 --- a/cms/static/js/views/modals/course_outline_modals.js +++ b/cms/static/js/views/modals/course_outline_modals.js @@ -1239,7 +1239,7 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', getContext: function() { return { optional_completion: this.model.get('optional_completion'), - optionalAncestor: this.model.get('ancestor_has_optional_completion') + optional_ancestor: this.model.get('ancestor_has_optional_completion') }; }, }) @@ -1267,7 +1267,7 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', editors: [] }; if (xblockInfo.isVertical()) { - editors = [StaffLockEditor, UnitAccessEditor, DiscussionEditor, OptionalCompletionEditor]; + editors = [StaffLockEditor, UnitAccessEditor, DiscussionEditor]; } else { tabs = [ { @@ -1282,10 +1282,10 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', } ]; if (xblockInfo.isChapter()) { - tabs[0].editors = [ReleaseDateEditor, OptionalCompletionEditor]; + tabs[0].editors = [ReleaseDateEditor]; tabs[1].editors = [StaffLockEditor]; } else if (xblockInfo.isSequential()) { - tabs[0].editors = [ReleaseDateEditor, GradingEditor, DueDateEditor, OptionalCompletionEditor]; + tabs[0].editors = [ReleaseDateEditor, GradingEditor, DueDateEditor]; tabs[1].editors = [ContentVisibilityEditor, ShowCorrectnessEditor]; if (course.get('self_paced') && course.get('is_custom_relative_dates_active')) { tabs[0].editors.push(SelfPacedDueDateEditor); @@ -1305,6 +1305,10 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', } } + if (course.get('completion_tracking_enabled')) { + tabs[0].editors.push(OptionalCompletionEditor) + } + /* globals course */ if (course.get('self_paced')) { editors = _.without(editors, ReleaseDateEditor, DueDateEditor); diff --git a/cms/templates/base.html b/cms/templates/base.html index 1e88505c7fde..a606038c59e3 100644 --- a/cms/templates/base.html +++ b/cms/templates/base.html @@ -8,6 +8,7 @@ ## Standard imports <%namespace name='static' file='static_content.html'/> <%! +from completion.waffle import ENABLE_COMPLETION_TRACKING_SWITCH from django.utils.translation import gettext as _ from cms.djangoapps.contentstore.config.waffle import CUSTOM_RELATIVE_DATES @@ -158,6 +159,7 @@ revision: "${context_course.location.branch | n, js_escaped_string}", self_paced: ${ context_course.self_paced | n, dump_js_escaped_json }, is_custom_relative_dates_active: ${CUSTOM_RELATIVE_DATES.is_enabled(context_course.id) | n, dump_js_escaped_json}, + completion_tracking_enabled: ${ENABLE_COMPLETION_TRACKING_SWITCH.is_enabled() | n, dump_js_escaped_json}, start: ${context_course.start | n, dump_js_escaped_json}, discussions_settings: ${context_course.discussions_settings | n, dump_js_escaped_json} }); diff --git a/cms/templates/js/optional-completion-editor.underscore b/cms/templates/js/optional-completion-editor.underscore index e6ae730124ff..9a7d55fe847a 100644 --- a/cms/templates/js/optional-completion-editor.underscore +++ b/cms/templates/js/optional-completion-editor.underscore @@ -4,7 +4,7 @@