Merge pull request #24391 from edx/mikix/assignments-complete

AA-225: Only consider scored items for past due assignments

common/lib/xmodule/xmodule/tests/test_vertical.py

@@ -60,6 +60,9 @@ class StubCompletionService(object):
         """
         return {candidate: self._completion_value for candidate in candidates}
 
+    def get_completable_children(self, node):
+        return node.get_children()
+
     def get_complete_on_view_delay_ms(self):
         """
         Return the completion-by-viewing delay in milliseconds.
@@ -72,15 +75,15 @@ class StubCompletionService(object):
     def vertical_is_complete(self, item):
         if item.scope_ids.block_type != 'vertical':
             raise ValueError('The passed in xblock is not a vertical type!')
-        return self._completion_value
+        return self._completion_value == 1 if self._enabled else None
 
 
 class BaseVerticalBlockTest(XModuleXmlImportTest):
     """
     Tests for the BaseVerticalBlock.
     """
-    test_html_1 = 'Test HTML 1'
-    test_html_2 = 'Test HTML 2'
+    test_html = 'Test HTML'
+    test_problem = 'Test Problem'
 
     def setUp(self):
         super(BaseVerticalBlockTest, self).setUp()
@@ -90,8 +93,8 @@ class BaseVerticalBlockTest(XModuleXmlImportTest):
         vertical = xml.VerticalFactory.build(parent=sequence)
 
         self.course = self.process_xml(course)
-        xml.HtmlFactory(parent=vertical, url_name='test-html-1', text=self.test_html_1)
-        xml.HtmlFactory(parent=vertical, url_name='test-html-2', text=self.test_html_2)
+        xml.HtmlFactory(parent=vertical, url_name='test-html', text=self.test_html)
+        xml.ProblemFactory(parent=vertical, url_name='test-problem', text=self.test_problem)
 
         self.course = self.process_xml(course)
         course_seq = self.course.get_children()[0]
@@ -103,8 +106,10 @@ class BaseVerticalBlockTest(XModuleXmlImportTest):
         self.vertical = course_seq.get_children()[0]
         self.vertical.xmodule_runtime = self.module_system
 
-        self.html1block = self.vertical.get_children()[0]
-        self.html2block = self.vertical.get_children()[1]
+        self.html_block = self.vertical.get_children()[0]
+        self.problem_block = self.vertical.get_children()[1]
+        self.problem_block.has_score = True
+        self.problem_block.graded = True
 
         self.username = "bilbo"
         self.default_context = {"bookmarked": False, "username": self.username}
@@ -151,8 +156,11 @@ class VerticalBlockTestCase(BaseVerticalBlockTest):
         html = self.module_system.render(
             self.vertical, view, self.default_context if context is None else context
         ).content
-        self.assertIn(self.test_html_1, html)
-        self.assertIn(self.test_html_2, html)
+        self.assertIn(self.test_html, html)
+        if view == STUDENT_VIEW:
+            self.assertIn(self.test_problem, html)
+        else:
+            self.assertNotIn(self.test_problem, html)
         self.assertIn("'due': datetime.datetime({year}, {month}, {day}".format(
             year=self.vertical.due.year, month=self.vertical.due.month, day=self.vertical.due.day), html)
         if view == STUDENT_VIEW:
@@ -161,9 +169,30 @@ class VerticalBlockTestCase(BaseVerticalBlockTest):
             self.assert_bookmark_info(self.assertNotIn, html)
         if context:
             self.assertIn("'subsection_format': '{}'".format(context['format']), html)
-            self.assertIn("'completed': {}".format(completion_value), html)
+            self.assertIn("'completed': {}".format(completion_value == 1), html)
             self.assertIn("'past_due': {}".format(self.vertical.due < now), html)
 
+    @ddt.data(True, False)
+    def test_render_problem_without_score(self, has_score):
+        """
+        Test the rendering of the student and public view.
+        """
+        self.module_system._services['bookmarks'] = Mock()
+        self.module_system._services['user'] = StubUserService()
+        self.module_system._services['completion'] = StubCompletionService(enabled=True, completion_value=0)
+
+        now = datetime.now(pytz.UTC)
+        self.vertical.due = now + timedelta(days=-1)
+        self.problem_block.has_score = has_score
+
+        html = self.module_system.render(self.vertical, STUDENT_VIEW, self.default_context).content
+        if has_score:
+            self.assertIn("'completed': False", html)
+            self.assertIn("'past_due': True", html)
+        else:
+            self.assertIn("'completed': None", html)
+            self.assertIn("'past_due': False", html)
+
     @ddt.unpack
     @ddt.data(
         (True, 0.9, True),
@@ -176,7 +205,7 @@ class VerticalBlockTestCase(BaseVerticalBlockTest):
         """
         Test that mark-completed-on-view-after-delay is only set for relevant child Xblocks.
         """
-        with patch.object(self.html1block, 'render') as mock_student_view:
+        with patch.object(self.html_block, 'render') as mock_student_view:
             self.module_system._services['completion'] = StubCompletionService(
                 enabled=completion_enabled,
                 completion_value=completion_value,
@@ -198,8 +227,8 @@ class VerticalBlockTestCase(BaseVerticalBlockTest):
             'is_unit_page': True
         }
         html = self.module_system.render(self.vertical, AUTHOR_VIEW, context).content
-        self.assertNotIn(self.test_html_1, html)
-        self.assertNotIn(self.test_html_2, html)
+        self.assertNotIn(self.test_html, html)
+        self.assertNotIn(self.test_problem, html)
 
         # Vertical should render reorderable children on the container page
         reorderable_items = set()
@@ -208,5 +237,5 @@ class VerticalBlockTestCase(BaseVerticalBlockTest):
             'reorderable_items': reorderable_items,
         }
         html = self.module_system.render(self.vertical, AUTHOR_VIEW, context).content
-        self.assertIn(self.test_html_1, html)
-        self.assertIn(self.test_html_2, html)
+        self.assertIn(self.test_html, html)
+        self.assertIn(self.test_problem, html)

common/lib/xmodule/xmodule/vertical_block.py

@@ -91,8 +91,8 @@ class VerticalBlock(SequenceFields, XModuleFields, StudioEditableBlock, XmlParse
                 'content': rendered_child.content
             })
 
-        completed = completion_service and completion_service.vertical_is_complete(self)
-        past_due = not completed and self.due and self.due < datetime.now(pytz.UTC)
+        completed = self.is_block_complete_for_assignments(completion_service)
+        past_due = completed is False and self.due and self.due < datetime.now(pytz.UTC)
         fragment_context = {
             'items': contents,
             'xblock_context': context,
@@ -224,3 +224,40 @@ class VerticalBlock(SequenceFields, XModuleFields, StudioEditableBlock, XmlParse
         xblock_body["content_type"] = "Sequence"
 
         return xblock_body
+
+    # So far, we only need this here. Move it somewhere more sensible if other bits of code want it too.
+    def is_block_complete_for_assignments(self, completion_service):
+        """
+        Considers a block complete only if all scored & graded leaf blocks are complete.
+
+        This is different from the normal `complete` flag because children of the block that are informative (like
+        readings or videos) do not count. We only care about actual homework content.
+
+        Compare with is_block_structure_complete_for_assignments in course_experience/utils.py, which does the same
+        calculation, but for a BlockStructure node and its children.
+
+        Returns:
+            True if complete
+            False if not
+            None if no assignments present or no completion info present (don't show any past-due or complete info)
+        """
+        if not completion_service or not completion_service.completion_tracking_enabled():
+            return None
+
+        children = completion_service.get_completable_children(self)
+        children_locations = [child.scope_ids.usage_id for child in children]
+        completions = completion_service.get_completions(children_locations)
+
+        all_complete = None
+        for child in children:
+            complete = completions[child.scope_ids.usage_id] == 1
+            graded = getattr(child, 'graded', False)
+            has_score = getattr(child, 'has_score', False)
+            weight = getattr(child, 'weight', 1)
+            scored = has_score and (weight is None or weight > 0)
+            if graded and scored:
+                if not complete:
+                    return False
+                all_complete = True
+
+        return all_complete

lms/djangoapps/courseware/courses.py

@@ -3,7 +3,6 @@ Functions for accessing and displaying courses within the
 courseware.
 """
 
-
 import logging
 from collections import defaultdict, namedtuple
 from datetime import datetime
@@ -12,7 +11,6 @@ import pytz
 import six
 from crum import get_current_request
 from django.conf import settings
-from django.db.models import Prefetch
 from django.http import Http404, QueryDict
 from django.urls import reverse
 from django.utils.translation import ugettext as _
@@ -25,7 +23,6 @@ from six import text_type
 from openedx.core.lib.cache_utils import request_cached
 
 import branding
-from course_modes.models import CourseMode
 from lms.djangoapps.courseware.access import has_access
 from lms.djangoapps.courseware.access_response import (
     AuthenticationRequiredAccessError,
@@ -60,6 +57,7 @@ from openedx.core.djangoapps.site_configuration import helpers as configuration_
 from openedx.core.lib.api.view_utils import LazySequence
 from openedx.features.course_duration_limits.access import AuditExpiredError
 from openedx.features.course_experience import RELATIVE_DATES_FLAG
+from openedx.features.course_experience.utils import is_block_structure_complete_for_assignments
 from static_replace import replace_static_urls
 from survey.utils import SurveyRequiredAccessError, check_survey_required_and_unanswered
 from util.date_utils import strftime_localized
@@ -550,7 +548,7 @@ def get_course_assignments(course_key, user, include_access=False):
             if assignment_released:
                 url = reverse('jump_to', args=[course_key, subsection_key])
 
-            complete = block_data.get_xblock_field(subsection_key, 'complete', False)
+            complete = is_block_structure_complete_for_assignments(block_data, subsection_key)
             past_due = not complete and due < now
             assignments.append(_Assignment(
                 subsection_key, title, url, due, contains_gated_content, complete, past_due, assignment_type

lms/djangoapps/courseware/tests/test_courses.py

@@ -11,6 +11,8 @@ import ddt
 import mock
 import pytz
 import six
+from completion.models import BlockCompletion
+from completion.test_utils import CompletionWaffleTestMixin
 from crum import set_current_request
 from django.conf import settings
 from django.test.client import RequestFactory
@@ -25,6 +27,7 @@ from lms.djangoapps.courseware.courses import (
     get_cms_block_link,
     get_cms_course_link,
     get_course_about_section,
+    get_course_assignments,
     get_course_by_id,
     get_course_chapter_ids,
     get_course_info_section,
@@ -454,3 +457,26 @@ class TestGetCourseChapters(ModuleStoreTestCase):
             course_chapter_ids,
             [six.text_type(child) for child in course.children]
         )
+
+
+class TestGetCourseAssignments(CompletionWaffleTestMixin, ModuleStoreTestCase):
+    """
+    Tests for the `get_course_assignments` function.
+    """
+
+    def test_completion_ignores_non_scored_items(self):
+        """
+        Test that we treat a sequential with incomplete (but not scored) items (like a video maybe) as complete.
+        """
+        course = CourseFactory()
+        chapter = ItemFactory(parent=course, category='chapter', graded=True, due=datetime.datetime.now())
+        sequential = ItemFactory(parent=chapter, category='sequential')
+        problem = ItemFactory(parent=sequential, category='problem', has_score=True)
+        ItemFactory(parent=sequential, category='video', has_score=False)
+
+        self.override_waffle_switch(True)
+        BlockCompletion.objects.submit_completion(self.user, problem.location, 1)
+
+        assignments = get_course_assignments(course.location.context_key, self.user, None)
+        self.assertEqual(len(assignments), 1)
+        self.assertTrue(assignments[0].complete)

lms/djangoapps/courseware/tests/test_views.py

@@ -3146,7 +3146,7 @@ class DatesTabTestCase(ModuleStoreTestCase):
                 format='Homework',
             )
             vertical = ItemFactory.create(category='vertical', parent_location=subsection.location)
-            ItemFactory.create(category='problem', parent_location=vertical.location)
+            ItemFactory.create(category='problem', parent_location=vertical.location, has_score=True)
 
         with patch('lms.djangoapps.courseware.views.views.get_enrollment') as mock_get_enrollment:
             mock_get_enrollment.return_value = {

openedx/features/course_experience/utils.py

@@ -299,14 +299,30 @@ def dates_banner_should_display(course_key, user):
     for section_key in block_data.get_children(course_usage_key):
         for subsection_key in block_data.get_children(section_key):
             subsection_due_date = block_data.get_xblock_field(subsection_key, 'due', None)
-            if subsection_due_date and (
-                not block_data.get_xblock_field(subsection_key, 'complete', False)
-                and block_data.get_xblock_field(subsection_key, 'graded', False)
-                and subsection_due_date < timezone.now()
-            ):
-                # Display the banner if the due date for an incomplete graded subsection
-                # has passed
+            if (subsection_due_date and subsection_due_date < timezone.now() and
+                    not is_block_structure_complete_for_assignments(block_data, subsection_key)):
+                # Display the banner if the due date for an incomplete graded subsection has passed
                 return True, block_data.get_xblock_field(subsection_key, 'contains_gated_content', False)
 
     # Don't display the banner if there were no missed deadlines
     return False, False
+
+
+def is_block_structure_complete_for_assignments(block_data, block_key):
+    """
+    Considers a block complete only if all scored & graded leaf blocks are complete.
+
+    This is different from the normal `complete` flag because children of the block that are informative (like
+    readings or videos) do not count. We only care about actual homework content.
+    """
+    children = block_data.get_children(block_key)
+    if children:
+        return all(is_block_structure_complete_for_assignments(block_data, child_key) for child_key in children)
+
+    complete = block_data.get_xblock_field(block_key, 'complete', False)
+    graded = block_data.get_xblock_field(block_key, 'graded', False)
+    has_score = block_data.get_xblock_field(block_key, 'has_score', False)
+    weight = block_data.get_xblock_field(block_key, 'weight', 1)
+    scored = has_score and (weight is None or weight > 0)
+
+    return complete or not graded or not scored