Merge pull request #26110 from edx/AA-497

[AA-497] Only set due dates on subsections that contain graded assignments with a score and a nonzero weight

openedx/core/djangoapps/course_date_signals/handlers.py

@@ -6,6 +6,9 @@ import logging
 from django.dispatch import receiver
 from edx_when.api import FIELDS_TO_EXTRACT, set_dates_for_course
 from six import text_type
+
+from common.lib.xmodule.xmodule.util.misc import is_xblock_an_assignment
+from openedx.core.lib.graph_traversals import get_children, leaf_filter, traverse_pre_order
 from xblock.fields import Scope
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import SignalHandler, modulestore
@@ -38,6 +41,17 @@ def _field_values(fields, xblock):
     return result
 
 
+def _has_assignment_blocks(item):
+    """
+    Check if a given block contains children that are assignments.
+    Assignments have graded, has_score and nonzero weight attributes.
+    """
+    return any(
+        is_xblock_an_assignment(block)
+        for block in traverse_pre_order(item, get_children, leaf_filter)
+    )
+
+
 def _gather_graded_items(root, due):
     items = [root]
     has_non_ora_scored_content = False
@@ -45,8 +59,12 @@ def _gather_graded_items(root, due):
     while items:
         next_item = items.pop()
         if next_item.graded:
+            # Sequentials can be marked as graded, while only containing ungraded problems
+            # To handle this case, we can look at the leaf blocks within a sequential
+            # and check that they are a graded assignment
+            # (if they have graded/has_score attributes and nonzero weight)
             # TODO: Once studio can manually set relative dates, we would need to manually check for them here
-            collected_items.append((next_item.location, {'due': due}))
+            collected_items.append((next_item.location, {'due': due if _has_assignment_blocks(next_item) else None}))
             # TODO: This is pretty gross, and should maybe be configurable in the future,
             # especially if we find ourselves needing more exceptions.
             has_non_ora_scored_content = (

openedx/core/djangoapps/course_date_signals/tests.py

@@ -2,6 +2,8 @@ from datetime import timedelta
 import ddt
 from unittest.mock import patch
 
+from openedx.core.djangoapps.course_date_signals.handlers import _gather_graded_items, _has_assignment_blocks
+from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from . import utils
@@ -40,3 +42,76 @@ class SelfPacedDueDatesTests(ModuleStoreTestCase):
             actual = [(idx, section.display_name, offset) for (idx, section, offset) in utils.spaced_out_sections(self.course)]
 
         self.assertEqual(actual, expected_sections)
+
+    def test_dates_for_ungraded_assignments(self):
+        """
+        _has_assignment_blocks should return true if the argument block
+        children leaf nodes include an assignment that is graded and scored
+        """
+        with modulestore().bulk_operations(self.course.id):
+            sequence = ItemFactory(parent=self.course, category="sequential")
+            vertical = ItemFactory(parent=sequence, category="vertical")
+            sequence = modulestore().get_item(sequence.location)
+            self.assertEqual(_has_assignment_blocks(sequence), False)
+
+            # Ungraded problems do not count as assignment blocks
+            ItemFactory.create(
+                parent=vertical,
+                category='problem',
+                graded=True,
+                weight=0,
+            )
+            sequence = modulestore().get_item(sequence.location)
+            self.assertEqual(_has_assignment_blocks(sequence), False)
+            ItemFactory.create(
+                parent=vertical,
+                category='problem',
+                graded=False,
+                weight=1,
+            )
+            sequence = modulestore().get_item(sequence.location)
+            self.assertEqual(_has_assignment_blocks(sequence), False)
+
+            # Method will return true after adding a graded, scored assignment block
+            ItemFactory.create(
+                parent=vertical,
+                category='problem',
+                graded=True,
+                weight=1,
+            )
+            sequence = modulestore().get_item(sequence.location)
+            self.assertEqual(_has_assignment_blocks(sequence), True)
+
+    def test_sequence_with_graded_and_ungraded_assignments(self):
+        """
+        _gather_graded_items should set a due date of None on ungraded problem blocks
+        even if the block has graded siblings in the sequence
+        """
+        with modulestore().bulk_operations(self.course.id):
+            sequence = ItemFactory(parent=self.course, category="sequential")
+            vertical = ItemFactory(parent=sequence, category="vertical")
+            sequence = modulestore().get_item(sequence.location)
+            ItemFactory.create(
+                parent=vertical,
+                category='problem',
+                graded=False,
+                weight=1,
+            )
+            ungraded_problem_2 = ItemFactory.create(
+                parent=vertical,
+                category='problem',
+                graded=True,
+                weight=0,
+            )
+            graded_problem_1 = ItemFactory.create(
+                parent=vertical,
+                category='problem',
+                graded=True,
+                weight=1,
+            )
+            expected_graded_items = [
+                (ungraded_problem_2.location, {'due': None}),
+                (graded_problem_1.location, {'due': 5}),
+            ]
+            sequence = modulestore().get_item(sequence.location)
+            self.assertCountEqual(_gather_graded_items(sequence, 5), expected_graded_items)

openedx/core/lib/graph_traversals.py

@@ -333,3 +333,23 @@ def _traverse_generic(
             # Keep track of whether or not the node was yielded so we
             # know whether or not to yield its children.
             yield_results[current_node] = should_yield_node
+
+
+def leaf_filter(block):
+    """
+    Filter for traversals to find leaf blocks
+    """
+    return (
+        block.location.block_type not in ('chapter', 'sequential', 'vertical') and
+        len(block.get_children()) == 0
+    )
+
+
+def get_children(parent):
+    """
+    Function for traversals to get the children of a block
+    """
+    if parent.has_children:
+        return parent.get_children()
+    else:
+        return []