diff --git a/common/lib/xmodule/xmodule/tests/test_vertical.py b/common/lib/xmodule/xmodule/tests/test_vertical.py index 3287d884d7b09a8b3c3b2e61e1e87eb0cb5ca005..117716b98269db5c8707b61b8504c518e0ff70a7 100644 --- a/common/lib/xmodule/xmodule/tests/test_vertical.py +++ b/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) diff --git a/common/lib/xmodule/xmodule/vertical_block.py b/common/lib/xmodule/xmodule/vertical_block.py index 9654d912e3c6232f608ecdea61078b054c0daaa5..9de73be343f894ba2caaa22974633a406f43d68e 100644 --- a/common/lib/xmodule/xmodule/vertical_block.py +++ b/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 diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 58db73568ed9695c6d25edad4195e392e3ff4b1c..91eea2ba1537603523d8c17bff52451636a6c29c 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/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 diff --git a/lms/djangoapps/courseware/tests/test_courses.py b/lms/djangoapps/courseware/tests/test_courses.py index 4d716d74210fe6bc785d2c12517ca634172f0081..35a8ec72053525d902536f13f888bfa990eb8d79 100644 --- a/lms/djangoapps/courseware/tests/test_courses.py +++ b/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) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index f70d8f6f6ea78255201c0156bb1bfdefc89a8dcf..0ec4836e009c43a29c2a3a472cf8c3cde57d0d93 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/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 = { diff --git a/openedx/features/course_experience/utils.py b/openedx/features/course_experience/utils.py index 9e5085e370bd1af8724ebadec892ad06517fedb6..5acb6dc4e469849658b6c258785bf05421f537c3 100644 --- a/openedx/features/course_experience/utils.py +++ b/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