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 332e9849f9..8747c8f84d 100644 --- a/cms/static/js/spec/views/pages/course_outline_spec.js +++ b/cms/static/js/spec/views/pages/course_outline_spec.js @@ -2422,26 +2422,6 @@ describe('CourseOutlinePage', function() { expect($('.modal-section .edit-discussion')).not.toExist(); }); - it('shows discussion settings if unit level discussions are enabled', function() { - getUnitStatus({}, {unit_level_discussions: true}); - outlinePage.$('.outline-unit .configure-button').click(); - expect($('.modal-section .edit-discussion')).toExist(); - }); - - it('marks checkbox as disabled', function() { - getUnitStatus({}, {unit_level_discussions: true}); - outlinePage.$('.outline-unit .configure-button').click(); - - var discussionCheckbox = $('#discussion_enabled'); - expect(discussionCheckbox).toExist(); - expect(discussionCheckbox.is(':checked')).toBeFalsy(); - }); - - it('marks checkbox as enabled', function() { - getUnitStatus({discussion_enabled: true}, {unit_level_discussions: true}); - outlinePage.$('.outline-unit .configure-button').click(); - expect($('#discussion_enabled').is(':checked')).toBeTruthy(); - }); }); verifyTypePublishable('unit', function(options) { diff --git a/cms/static/js/views/modals/course_outline_modals.js b/cms/static/js/views/modals/course_outline_modals.js index 7a4f7485b0..2036de70e1 100644 --- a/cms/static/js/views/modals/course_outline_modals.js +++ b/cms/static/js/views/modals/course_outline_modals.js @@ -941,6 +941,7 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', afterRender: function() { AbstractEditor.prototype.afterRender.call(this); this.setStatus(this.currentValue()); + this.showTipText(); }, currentValue: function() { @@ -948,6 +949,21 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', return discussionEnabled === true || discussionEnabled === 'enabled'; }, + showTipText: function() { + if (this.model.get('published')) { + $('.un-published-tip').hide() + } else { + $('.un-published-tip').show() + } + let enabledForGraded = course.get('discussions_settings').enable_graded_units + if (this.model.get('graded') && !enabledForGraded) { + $('#discussion_enabled').prop('disabled', true); + $('.graded-tip').show() + } else { + $('.graded-tip').hide() + } + }, + setStatus: function(value) { this.$('#discussion_enabled').prop('checked', value); }, diff --git a/cms/templates/js/discussion-editor.underscore b/cms/templates/js/discussion-editor.underscore index c0fff944a9..cbc734e9bf 100644 --- a/cms/templates/js/discussion-editor.underscore +++ b/cms/templates/js/discussion-editor.underscore @@ -7,5 +7,13 @@ <%- gettext('Enable discussion') %> +
+

+ <%- gettext('Topics for unpublished units would not be created') %> +

+

+ <%- gettext('Please enable discussions for graded units from course authoring app') %> +

+
diff --git a/openedx/core/djangoapps/discussions/tasks.py b/openedx/core/djangoapps/discussions/tasks.py index 1985da1c9a..411205eb30 100644 --- a/openedx/core/djangoapps/discussions/tasks.py +++ b/openedx/core/djangoapps/discussions/tasks.py @@ -50,31 +50,56 @@ def update_discussions_settings_from_course(course_key: CourseKey) -> CourseDisc supports_in_context = discussions_config.supports_in_context_discussions() provider_type = discussions_config.provider_type - def iter_discussable_units(): - # Start at 99 so that the initial increment starts it at 100. - # This leaves the first 100 slots for the course wide topics, which is only a concern if there are more - # than that many. - idx = 99 - for section in course.get_children(): - if section.location.block_type != "chapter": - continue - for subsection in section.get_children(): - if subsection.location.block_type != "sequential": - continue - for unit in subsection.get_children(): - if unit.location.block_type != 'vertical': - continue - # Increment index even for skipped units so that the index is more stable and won't change - # if settings change, only if a unit is added or removed. + with store.branch_setting(ModuleStoreEnum.Branch.published_only, course_key): + course = store.get_course(course_key) + enable_in_context = discussions_config.enable_in_context + provider_config = discussions_config.plugin_configuration + unit_level_visibility = discussions_config.unit_level_visibility + enable_graded_units = discussions_config.enable_graded_units + + contexts = [] + if supports_in_context: + sorted_topics = sorted( + course.discussion_topics.items(), + key=lambda item: item[1].get("sort_key", item[0]), + ) + contexts = [] + for idx, (topic_name, topic_config) in enumerate(sorted_topics): + if topic_config.get('id', None): + context = DiscussionTopicContext( + title=topic_name, + external_id=topic_config.get('id', None), + ordering=idx + ) + contexts.append(context) + if enable_in_context: + contexts.extend(list(get_discussable_units(course, enable_graded_units))) + config_data = CourseDiscussionConfigurationData( + course_key=course_key, + enable_in_context=enable_in_context, + enable_graded_units=enable_graded_units, + unit_level_visibility=unit_level_visibility, + provider_type=provider_type, + plugin_configuration=provider_config, + contexts=contexts, + ) + return config_data + + +def get_discussable_units(course, enable_graded_units): + """ + Get all the units in the course that are discussable. + """ + idx = 99 + store = modulestore() + for section in get_sections(course): + for subsection in get_subsections(section): + with store.bulk_operations(course.id, emit_signals=False): + for unit in get_units(subsection): idx += 1 - # If unit-level visibility is enabled and the unit doesn't have discussion enabled, skip it. - if unit_level_visibility and not getattr(unit, "discussion_enabled", False): - continue - # If the unit is in a graded section and graded sections aren't enabled skip it. - if subsection.graded and not enable_graded_units: - continue - # If the unit is an exam, skip it. - if subsection.is_practice_exam or subsection.is_proctored_enabled or subsection.is_time_limited: + if not is_discussable_unit(unit, store, enable_graded_units, subsection): + unit.discussion_enabled = False + store.update_item(unit, unit.published_by, emit_signals=False) continue yield DiscussionTopicContext( usage_key=unit.location, @@ -88,39 +113,51 @@ def update_discussions_settings_from_course(course_key: CourseKey) -> CourseDisc }, ) - with store.branch_setting(ModuleStoreEnum.Branch.published_only, course_key): - course = store.get_course(course_key) - enable_in_context = discussions_config.enable_in_context - provider_config = discussions_config.plugin_configuration - unit_level_visibility = discussions_config.unit_level_visibility - enable_graded_units = discussions_config.enable_graded_units - contexts = [] - if supports_in_context: - sorted_topics = sorted( - course.discussion_topics.items(), - key=lambda item: item[1].get("sort_key", item[0]), - ) - contexts = [ - DiscussionTopicContext( - title=topic_name, - external_id=topic_config.get('id', None), - ordering=idx, - ) - for idx, (topic_name, topic_config) in enumerate(sorted_topics) - if topic_config.get('id', None) - ] - if enable_in_context: - contexts.extend(list(iter_discussable_units())) - config_data = CourseDiscussionConfigurationData( - course_key=course_key, - enable_in_context=enable_in_context, - enable_graded_units=enable_graded_units, - unit_level_visibility=unit_level_visibility, - provider_type=provider_type, - plugin_configuration=provider_config, - contexts=contexts, - ) - return config_data + +def get_sections(course): + """ + Get sections for given course + """ + for section in course.get_children(): + if section.location.block_type == "chapter": + yield section + + +def get_subsections(section): + """ + Get subsections for given section + """ + for subsection in section.get_children(): + if subsection.location.block_type == "sequential": + yield subsection + + +def get_units(subsection): + """ + Get units for given subsection + """ + for unit in subsection.get_children(): + if unit.location.block_type == 'vertical': + yield unit + + +def is_discussable_unit(unit, store, enable_graded_units, subsection): + """ + Check if unit should have discussion's topic + """ + if not store.has_published_version(unit): + return False + + if not getattr(unit, "discussion_enabled", False): + return False + + if subsection.graded and not enable_graded_units: + return False + + if subsection.is_practice_exam or subsection.is_proctored_enabled or subsection.is_time_limited: + return False + + return True def update_unit_discussion_state_from_discussion_blocks(course_key: CourseKey, user_id: int, force=False) -> None: diff --git a/openedx/core/djangoapps/discussions/tests/test_tasks.py b/openedx/core/djangoapps/discussions/tests/test_tasks.py index 58514ca3c5..fa04713ae4 100644 --- a/openedx/core/djangoapps/discussions/tests/test_tasks.py +++ b/openedx/core/djangoapps/discussions/tests/test_tasks.py @@ -162,10 +162,10 @@ class UpdateDiscussionsSettingsFromCourseTestCase(ModuleStoreTestCase, Discussio ({}, 3, {"Unit", "Discussable Unit"}, {"Graded Unit", "Non-Discussable Unit", "Discussable Graded Unit", "Non-Discussable Graded Unit"}), ({"enable_in_context": False}, 1, set(), {"Unit", "Graded Unit"}), - ({"unit_level_visibility": False, "enable_graded_units": False}, 4, - {"Unit", "Discussable Unit", "Non-Discussable Unit"}, + ({"unit_level_visibility": False, "enable_graded_units": False}, 3, + {"Unit", "Discussable Unit"}, {"Graded Unit"}), - ({"unit_level_visibility": False, "enable_graded_units": True}, 7, + ({"unit_level_visibility": False, "enable_graded_units": True}, 5, {"Unit", "Graded Unit", "Discussable Graded Unit"}, set()), ({"enable_graded_units": True}, 5, {"Discussable Unit", "Discussable Graded Unit", "Graded Unit"},