feat: added user messages and backed now uses discussion_enabled flag (…

…#31716)

* refactor: simplified tasks.py for discussions

* fix: do not create a topic for the unpublished unit

* feat: added user messages and backed now uses discussion_enabled flag

* fix: update default for discussion_enabled flag

* feat: removed redundant tests and fixes

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) {

cms/static/js/views/modals/course_outline_modals.js

@@ -941,13 +941,29 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview',
         afterRender: function() {
             AbstractEditor.prototype.afterRender.call(this);
             this.setStatus(this.currentValue());
+            this.showTipText();
         },
 
         currentValue: function() {
             var discussionEnabled = this.model.get('discussion_enabled');
             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);
         },

cms/templates/js/discussion-editor.underscore

@@ -7,5 +7,13 @@
         <input type="checkbox" id="discussion_enabled" name="discussion_enabled" class="input input-checkbox" />
         <%- gettext('Enable discussion') %>
      </label>
+<div class="ml-5">
+    <p class="un-published-tip tip tip-inline">
+        <%- gettext('Topics for unpublished units would not be created') %>
+    </p>
+    <p class="graded-tip tip tip-inline">
+        <%- gettext('Please enable discussions for graded units from course authoring app') %>
+    </p>
+</div>
 </div>
 </form>

openedx/core/djangoapps/discussions/tasks.py

@@ -50,67 +50,30 @@ 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.
-                    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:
-                        continue
-                    yield DiscussionTopicContext(
-                        usage_key=unit.location,
-                        title=unit.display_name,
-                        group_id=None,
-                        ordering=idx,
-                        context={
-                            "section": section.display_name,
-                            "subsection": subsection.display_name,
-                            "unit": unit.display_name,
-                        },
-                    )
-
     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)
-            ]
+            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(iter_discussable_units()))
+                contexts.extend(list(get_discussable_units(course, enable_graded_units)))
         config_data = CourseDiscussionConfigurationData(
             course_key=course_key,
             enable_in_context=enable_in_context,
@@ -123,6 +86,80 @@ def iter_discussable_units():
     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 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,
+                        title=unit.display_name,
+                        group_id=None,
+                        ordering=idx,
+                        context={
+                            "section": section.display_name,
+                            "subsection": subsection.display_name,
+                            "unit": unit.display_name,
+                        },
+                    )
+
+
+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:
     """
     Migrate existing courses to the new mechanism for linking discussion to units.

openedx/core/djangoapps/discussions/tests/test_tasks.py

@@ -162,10 +162,10 @@ def test_topics_contexts(self):
         ({}, 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"},