From f544a4825dffeef684401f4a7cf0f68b9c4b3549 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Thu, 29 Feb 2024 11:13:33 -0400 Subject: [PATCH] feat: make hide from TOC a visibility section setting (#33952) Exposes the hide_from_toc xblock attribute so course authors can configure it as a section visibility option in Studio. Before this change, the Hide from TOC functionality was mainly used by OLX components. Hence, it wasn't available for configuration through the Studio UI. Still, its implementation existed in the platform and could be used by setting the attribute: hide_from_toc=true as part of the OLX definition. Ref: https://openedx.atlassian.net/wiki/spaces/OEPM/pages/3853975595/Feature+Enhancement+Proposal+Hide+Sections+from+course+outline --- .../rest_api/v0/serializers/xblock.py | 1 + .../contentstore/tests/test_contentstore.py | 17 ++++++++ .../xblock_storage_handlers/view_handlers.py | 29 ++++++++++--- cms/static/js/models/xblock_info.js | 8 ++++ .../spec/views/pages/course_outline_spec.js | 26 +++++++++--- .../js/views/modals/course_outline_modals.js | 41 +++++++++++++++++-- .../js/views/pages/container_subviews.js | 1 + cms/static/js/views/utils/xblock_utils.js | 8 +++- cms/static/js/views/xblock_outline.js | 1 + cms/static/sass/elements/_modules.scss | 22 ++++++++++ .../partials/cms/theme/_variables-v1.scss | 1 + cms/static/sass/views/_container.scss | 9 ++++ cms/templates/js/course-outline.underscore | 17 +++++++- cms/templates/js/staff-lock-editor.underscore | 35 ++++++++++++++-- xmodule/modulestore/inheritance.py | 7 ++++ 15 files changed, 205 insertions(+), 18 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v0/serializers/xblock.py b/cms/djangoapps/contentstore/rest_api/v0/serializers/xblock.py index 4549326c96..e95a76e918 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/serializers/xblock.py +++ b/cms/djangoapps/contentstore/rest_api/v0/serializers/xblock.py @@ -77,3 +77,4 @@ class XblockSerializer(StrictSerializer): target_index = serializers.IntegerField(required=False, allow_null=True) boilerplate = serializers.JSONField(required=False, allow_null=True) staged_content = serializers.CharField(required=False, allow_null=True) + hide_from_toc = serializers.BooleanField(required=False, allow_null=True) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 9f90840156..e09ff48834 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1445,6 +1445,23 @@ class ContentStoreTest(ContentStoreTestCase): ).replace('REPLACE', r'([0-9]|[a-f]){3,}') self.assertRegex(data['locator'], retarget) + @ddt.data(True, False) + def test_hide_xblock_from_toc_via_handler(self, hide_from_toc): + """Test that the hide_from_toc field can be set via the xblock_handler.""" + course = CourseFactory.create() + sequential = BlockFactory.create(parent_location=course.location) + data = { + "metadata": { + "hide_from_toc": hide_from_toc + } + } + + response = self.client.ajax_post(get_url("xblock_handler", sequential.location), data) + sequential = self.store.get_item(sequential.location) + + self.assertEqual(response.status_code, 200) + self.assertEqual(hide_from_toc, sequential.hide_from_toc) + def test_capa_block(self): """Test that a problem treats markdown specially.""" course = CourseFactory.create() diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index 44b89aa3a5..92f991b470 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -1089,6 +1089,8 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements "group_access": xblock.group_access, "user_partitions": user_partitions, "show_correctness": xblock.show_correctness, + "hide_from_toc": xblock.hide_from_toc, + "enable_hide_from_toc_ui": settings.FEATURES.get("ENABLE_HIDE_FROM_TOC_UI", False), } ) @@ -1217,6 +1219,15 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements else: xblock_info["staff_only_message"] = False + if xblock_info["hide_from_toc"]: + xblock_info["hide_from_toc_message"] = True + elif child_info and child_info["children"]: + xblock_info["hide_from_toc_message"] = all( + child["hide_from_toc_message"] for child in child_info["children"] + ) + else: + xblock_info["hide_from_toc_message"] = False + # If the ENABLE_TAGGING_TAXONOMY_LIST_PAGE feature flag is enabled, we show the "Manage Tags" options if use_tagging_taxonomy_list_page(): xblock_info["use_tagging_taxonomy_list_page"] = True @@ -1345,6 +1356,7 @@ class VisibilityState: needs_attention = "needs_attention" staff_only = "staff_only" gated = "gated" + hide_from_toc = "hide_from_toc" def _compute_visibility_state( @@ -1355,6 +1367,8 @@ def _compute_visibility_state( """ if xblock.visible_to_staff_only: return VisibilityState.staff_only + elif xblock.hide_from_toc: + return VisibilityState.hide_from_toc elif is_unit_with_changes: # Note that a unit that has never been published will fall into this category, # as well as previously published units with draft content. @@ -1362,22 +1376,27 @@ def _compute_visibility_state( is_unscheduled = xblock.start == DEFAULT_START_DATE is_live = is_course_self_paced or datetime.now(UTC) > xblock.start - if child_info and child_info.get("children", []): + if child_info and child_info.get("children", []): # pylint: disable=too-many-nested-blocks all_staff_only = True all_unscheduled = True all_live = True + all_hide_from_toc = True for child in child_info["children"]: child_state = child["visibility_state"] if child_state == VisibilityState.needs_attention: return child_state elif not child_state == VisibilityState.staff_only: all_staff_only = False - if not child_state == VisibilityState.unscheduled: - all_unscheduled = False - if not child_state == VisibilityState.live: - all_live = False + if not child_state == VisibilityState.hide_from_toc: + all_hide_from_toc = False + if not child_state == VisibilityState.unscheduled: + all_unscheduled = False + if not child_state == VisibilityState.live: + all_live = False if all_staff_only: return VisibilityState.staff_only + elif all_hide_from_toc: + return VisibilityState.hide_from_toc elif all_unscheduled: return ( VisibilityState.unscheduled diff --git a/cms/static/js/models/xblock_info.js b/cms/static/js/models/xblock_info.js index 49812a6c9d..2b82cf72b1 100644 --- a/cms/static/js/models/xblock_info.js +++ b/cms/static/js/models/xblock_info.js @@ -177,6 +177,14 @@ define( * List of tags of the unit. This list is managed by the content_tagging module. */ tags: null, + /** + * True if the xblock is not visible to students only via links. + */ + hide_from_toc: null, + /** + * True iff this xblock should display a "Contains staff only content" message. + */ + hide_from_toc_message: null, }, initialize: function() { diff --git a/cms/static/js/spec/views/pages/course_outline_spec.js b/cms/static/js/spec/views/pages/course_outline_spec.js index a230d17797..7253938b31 100644 --- a/cms/static/js/spec/views/pages/course_outline_spec.js +++ b/cms/static/js/spec/views/pages/course_outline_spec.js @@ -42,6 +42,8 @@ describe('CourseOutlinePage', function() { user_partition_info: {}, highlights_enabled: true, highlights_enabled_for_messaging: false, + hide_from_toc: false, + enable_hide_from_toc_ui: true }, options, {child_info: {children: children}}); }; @@ -68,7 +70,9 @@ describe('CourseOutlinePage', function() { show_review_rules: true, user_partition_info: {}, highlights_enabled: true, - highlights_enabled_for_messaging: false + highlights_enabled_for_messaging: false, + hide_from_toc: false, + enable_hide_from_toc_ui: true }, options, {child_info: {children: children}}); }; @@ -93,7 +97,9 @@ describe('CourseOutlinePage', function() { group_access: {}, user_partition_info: {}, highlights: [], - highlights_enabled: true + highlights_enabled: true, + hide_from_toc: false, + enable_hide_from_toc_ui: true }, options, {child_info: {children: children}}); }; @@ -123,7 +129,9 @@ describe('CourseOutlinePage', function() { }, user_partitions: [], group_access: {}, - user_partition_info: {} + user_partition_info: {}, + hide_from_toc: false, + enable_hide_from_toc_ui: true }, options, {child_info: {children: children}}); }; @@ -141,7 +149,9 @@ describe('CourseOutlinePage', function() { edited_by: 'MockUser', user_partitions: [], group_access: {}, - user_partition_info: {} + user_partition_info: {}, + hide_from_toc: false, + enable_hide_from_toc_ui: true }, options); }; @@ -1214,7 +1224,9 @@ describe('CourseOutlinePage', function() { is_practice_exam: false, is_proctored_exam: false, default_time_limit_minutes: 150, - hide_after_due: true + hide_after_due: true, + hide_from_toc: false, + enable_hide_from_toc_ui: true, }, [ createMockVerticalJSON({ has_changes: true, @@ -1397,6 +1409,7 @@ describe('CourseOutlinePage', function() { default_time_limit_minutes: 150, hide_after_due: true, is_onboarding_exam: false, + hide_from_toc: null, } }); expect(requests[0].requestHeaders['X-HTTP-Method-Override']).toBe('PATCH'); @@ -2240,6 +2253,8 @@ describe('CourseOutlinePage', function() { is_practice_exam: false, is_proctored_exam: false, default_time_limit_minutes: null, + hide_from_toc: false, + enable_hide_from_toc_ui: true, }, [ createMockVerticalJSON({ has_changes: true, @@ -2521,6 +2536,7 @@ describe('CourseOutlinePage', function() { publish: 'republish', metadata: { visible_to_staff_only: null, + hide_from_toc: null } }); }) diff --git a/cms/static/js/views/modals/course_outline_modals.js b/cms/static/js/views/modals/course_outline_modals.js index f79e65d207..f52340dea8 100644 --- a/cms/static/js/views/modals/course_outline_modals.js +++ b/cms/static/js/views/modals/course_outline_modals.js @@ -314,6 +314,8 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', var isProctoredExam = xblockInfo.get('is_proctored_exam'); var isPracticeExam = xblockInfo.get('is_practice_exam'); var isOnboardingExam = xblockInfo.get('is_onboarding_exam'); + var enableHideFromTOCUI = xblockInfo.get('enable_hide_from_toc_ui'); + var hideFromTOC = xblockInfo.get('hide_from_toc'); var html = this.template($.extend({}, { xblockInfo: xblockInfo, xblockType: this.options.xblockType, @@ -323,6 +325,8 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', isProctoredExam: isProctoredExam, isPracticeExam: isPracticeExam, isOnboardingExam: isOnboardingExam, + enableHideFromTOCUI: enableHideFromTOCUI, + hideFromTOC: hideFromTOC, isTimedExam: isTimeLimited && !( isProctoredExam || isPracticeExam || isOnboardingExam ), @@ -798,6 +802,10 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', return this.model.get('ancestor_has_staff_lock'); }, + isModelHiddenFromTOC: function() { + return this.model.get('hide_from_toc'); + }, + getContext: function() { return { hasExplicitStaffLock: this.isModelLocked(), @@ -812,6 +820,8 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', afterRender: function() { AbstractVisibilityEditor.prototype.afterRender.call(this); this.setLock(this.isModelLocked()); + this.setHideFromTOC(this.isModelHiddenFromTOC()); + this.setVisibleToLearners(this.isVisibleToLearners()); }, setLock: function(value) { @@ -822,8 +832,24 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', return this.$('#staff_lock').is(':checked'); }, + setHideFromTOC: function(value) { + this.$('#hide_from_toc').prop('checked', value); + }, + + setVisibleToLearners: function(value) { + this.$('#visible_to_learners').prop('checked', value); + }, + + isVisibleToLearners: function() { + return this.$('#staff_lock').is(':not(:checked)') && this.$('#hide_from_toc').is(':not(:checked)'); + }, + + isHiddenFromTOC: function() { + return this.$('#hide_from_toc').is(':checked'); + }, + hasChanges: function() { - return this.isModelLocked() !== this.isLocked(); + return this.isModelLocked() !== this.isLocked() || this.isModelHiddenFromTOC() !== this.isHiddenFromTOC(); }, getRequestData: function() { @@ -831,7 +857,8 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', return { publish: 'republish', metadata: { - visible_to_staff_only: this.isLocked() ? true : null + visible_to_staff_only: this.isLocked() || null, + hide_from_toc: this.isHiddenFromTOC() || null } }; } else { @@ -1055,12 +1082,20 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', if (this.currentVisibility() === 'staff_only') { metadata.visible_to_staff_only = true; metadata.hide_after_due = null; + metadata.hide_from_toc = null; } else if (this.currentVisibility() === 'hide_after_due') { metadata.visible_to_staff_only = null; metadata.hide_after_due = true; - } else { + metadata.hide_from_toc = null; + } else if (this.currentVisibility() === 'hide_from_toc'){ metadata.visible_to_staff_only = null; metadata.hide_after_due = null; + metadata.hide_from_toc = true; + } + else { + metadata.visible_to_staff_only = null; + metadata.hide_after_due = null; + metadata.hide_from_toc = null; } return { diff --git a/cms/static/js/views/pages/container_subviews.js b/cms/static/js/views/pages/container_subviews.js index 7ea09eff08..2fe33df88e 100644 --- a/cms/static/js/views/pages/container_subviews.js +++ b/cms/static/js/views/pages/container_subviews.js @@ -149,6 +149,7 @@ function($, _, gettext, BaseView, ViewUtils, XBlockViewUtils, MoveXBlockUtils, H releaseDate: this.model.get('release_date'), releaseDateFrom: this.model.get('release_date_from'), hasExplicitStaffLock: this.model.get('has_explicit_staff_lock'), + hideFromTOC: this.model.get('hide_from_toc'), staffLockFrom: this.model.get('staff_lock_from'), enableCopyUnit: this.model.get('enable_copy_paste_units'), course: window.course, diff --git a/cms/static/js/views/utils/xblock_utils.js b/cms/static/js/views/utils/xblock_utils.js index 9abe0866ed..e955a0a3ad 100644 --- a/cms/static/js/views/utils/xblock_utils.js +++ b/cms/static/js/views/utils/xblock_utils.js @@ -29,6 +29,8 @@ function($, _, gettext, ViewUtils, ModuleUtils, XBlockInfo, StringUtils) { * * staffOnly - all of the block's content is to be shown to staff only * Note: staff only items do not affect their parent's state. + * + * hideFromTOC - all of the block's content is to be hidden from the table of contents. */ VisibilityState = { live: 'live', @@ -36,7 +38,8 @@ function($, _, gettext, ViewUtils, ModuleUtils, XBlockInfo, StringUtils) { unscheduled: 'unscheduled', needsAttention: 'needs_attention', staffOnly: 'staff_only', - gated: 'gated' + gated: 'gated', + hideFromTOC: 'hide_from_toc' }; /** @@ -310,6 +313,9 @@ function($, _, gettext, ViewUtils, ModuleUtils, XBlockInfo, StringUtils) { if (visibilityState === VisibilityState.staffOnly) { return 'is-staff-only'; } + if (visibilityState === VisibilityState.hideFromTOC) { + return 'is-hidden-from-toc'; + } if (visibilityState === VisibilityState.gated) { return 'is-gated'; } diff --git a/cms/static/js/views/xblock_outline.js b/cms/static/js/views/xblock_outline.js index 4e826025b9..d5a12e6f0c 100644 --- a/cms/static/js/views/xblock_outline.js +++ b/cms/static/js/views/xblock_outline.js @@ -112,6 +112,7 @@ function($, _, gettext, BaseView, ViewUtils, XBlockViewUtils, XBlockStringFieldE includesChildren: this.shouldRenderChildren(), hasExplicitStaffLock: this.model.get('has_explicit_staff_lock'), staffOnlyMessage: this.model.get('staff_only_message'), + hideFromTOCMessage: this.model.get('hide_from_toc_message'), course: course, enableCopyPasteUnits: this.model.get("enable_copy_paste_units"), // ENABLE_COPY_PASTE_UNITS waffle flag useTaggingTaxonomyListPage: this.model.get("use_tagging_taxonomy_list_page"), // ENABLE_TAGGING_TAXONOMY_LIST_PAGE waffle flag diff --git a/cms/static/sass/elements/_modules.scss b/cms/static/sass/elements/_modules.scss index 4f7532c915..29a2989c1a 100644 --- a/cms/static/sass/elements/_modules.scss +++ b/cms/static/sass/elements/_modules.scss @@ -507,6 +507,18 @@ $outline-indent-width: $baseline; } } + // CASE: is hidden from TOC + &.is-hidden-from-toc { + // needed to make sure direct children only + > .section-status, + > .subsection-status, + > .unit-status { + .status-message .icon { + color: $color-hide-from-toc; + } + } + } + // CASE: has gated content &.is-gated { @@ -603,6 +615,11 @@ $outline-indent-width: $baseline; border-left-color: $color-staff-only; } + // CASE: is hidden from TOC + &.is-hidden-from-toc { + border-left-color: $color-hide-from-toc; + } + // CASE: has gated content &.is-gated { border-left-color: $color-gated; @@ -698,6 +715,11 @@ $outline-indent-width: $baseline; border-left-color: $color-staff-only; } + // CASE: is hidden from TOC + &.is-hidden-from-toc { + border-left-color: $color-hide-from-toc; + } + // CASE: is presented for gated &.is-gated { border-left-color: $color-gated; diff --git a/cms/static/sass/partials/cms/theme/_variables-v1.scss b/cms/static/sass/partials/cms/theme/_variables-v1.scss index 96777b34f2..c2ff073b94 100644 --- a/cms/static/sass/partials/cms/theme/_variables-v1.scss +++ b/cms/static/sass/partials/cms/theme/_variables-v1.scss @@ -218,6 +218,7 @@ $color-ready: $green !default; $color-warning: $orange-l2 !default; $color-error: $red-l2 !default; $color-staff-only: $black !default; +$color-hide-from-toc: $black !default; $color-gated: $black !default; $color-heading-base: $gray-d2 !default; diff --git a/cms/static/sass/views/_container.scss b/cms/static/sass/views/_container.scss index f28ac839c5..5a674ba6e0 100644 --- a/cms/static/sass/views/_container.scss +++ b/cms/static/sass/views/_container.scss @@ -155,6 +155,15 @@ } } + // CASE: is hidden from TOC + &.is-hidden-from-toc{ + @extend %bar-module-black; + + &.is-scheduled .wrapper-release .copy { + text-decoration: line-through; + } + } + // CASE: content is gated &.is-gated { @extend %bar-module-black; diff --git a/cms/templates/js/course-outline.underscore b/cms/templates/js/course-outline.underscore index c62979bced..69b33f05ec 100644 --- a/cms/templates/js/course-outline.underscore +++ b/cms/templates/js/course-outline.underscore @@ -22,7 +22,13 @@ var addStatusMessage = function (statusType, message) { statusIconClass = 'fa-warning'; } else if (statusType === 'staff-only' || statusType === 'gated') { statusIconClass = 'fa-lock'; - } else if (statusType === 'partition-groups') { + } else if(statusType === 'hide-from-toc-chapter') { + statusIconClass = 'fa-eye-slash'; + } + else if(statusType === 'hide-from-toc-sequence') { + statusIconClass = 'fa-ban'; + } + else if (statusType === 'partition-groups') { statusIconClass = 'fa-eye'; } @@ -49,6 +55,15 @@ if (staffOnlyMessage) { messageType = 'staff-only'; messageText = gettext('Contains staff only content'); addStatusMessage(messageType, messageText); +} else if (hideFromTOCMessage && !xblockInfo.isVertical()){ + if (xblockInfo.isChapter()) { + messageText = gettext('Hidden in the course outline, but sections are accessible for your learners through their separate link.'); + messageType = 'hide-from-toc-chapter'; + } else { + messageText = gettext('Subsections are not navigable between each other, they can only be accessed through their link.'); + messageType = 'hide-from-toc-sequence'; + } + addStatusMessage(messageType, messageText); } else { if (visibilityState === 'needs_attention' && xblockInfo.isVertical()) { messageType = 'warning'; diff --git a/cms/templates/js/staff-lock-editor.underscore b/cms/templates/js/staff-lock-editor.underscore index ff4417fbf3..f632001a8b 100644 --- a/cms/templates/js/staff-lock-editor.underscore +++ b/cms/templates/js/staff-lock-editor.underscore @@ -6,11 +6,39 @@ <%- gettext('Section Visibility') %> <% } %> +
+<% if (enableHideFromTOCUI) { %> +
+ +

<%- gettext('It is visible to learners, ensuring that all learners can view its contents.') %>

+
+
+ <% if (!xblockInfo.isVertical()) { %> + +

<%- gettext('It is intentionally hidden from standard navigation, ensuring that only individuals with the link can view its contents.') %>

+ <% if (hideFromTOC) { %> +

+ <%- gettext('If you make this section visible in the table of content, learners will be able to see its content.') %> +

+ <% } %> + <% } %> +
+<% } %>
+ + <% if (enableHideFromTOCUI) { %> +

<%- gettext('It is intentionally hidden from learners, ensuring that only the course staff can view its contents.') %>

+ <% } %> <% if (hasExplicitStaffLock && !ancestorLocked) { %>

<% if (xblockInfo.isVertical()) { %> @@ -20,6 +48,7 @@ <%- interpolate(message, { xblockType: xblockType }, true) %> <% } %>

- <% } %> + <% } %> +
diff --git a/xmodule/modulestore/inheritance.py b/xmodule/modulestore/inheritance.py index 489365e80e..4c5a14b769 100644 --- a/xmodule/modulestore/inheritance.py +++ b/xmodule/modulestore/inheritance.py @@ -240,6 +240,13 @@ class InheritanceMixin(XBlockMixin): scope=Scope.settings ) + hide_from_toc = Boolean( + display_name=_("Hide from Table of Contents"), + help=_("Enter true or false. If true, this block will be hidden from the Table of Contents."), + default=False, + scope=Scope.settings + ) + @property def close_date(self): """