From f76a099e2dbbb2b799cb500eb655807c72fa1085 Mon Sep 17 00:00:00 2001 From: "J. Cliff Dyer" Date: Thu, 26 Oct 2017 11:17:14 -0400 Subject: [PATCH] Skip completion of non-default scorable blocks. If a scorable block either has a custom completion strategy, or is marked as excluded from completion, don't record a completion when its score is updated. --- lms/djangoapps/completion/handlers.py | 11 ++- .../completion/tests/test_handlers.py | 86 +++++++++++++++---- 2 files changed, 80 insertions(+), 17 deletions(-) diff --git a/lms/djangoapps/completion/handlers.py b/lms/djangoapps/completion/handlers.py index aded416de8..f7b70e20ad 100644 --- a/lms/djangoapps/completion/handlers.py +++ b/lms/djangoapps/completion/handlers.py @@ -7,8 +7,10 @@ from __future__ import absolute_import, division, print_function, unicode_litera from django.contrib.auth.models import User from django.dispatch import receiver -from opaque_keys.edx.keys import CourseKey, UsageKey from lms.djangoapps.grades.signals.signals import PROBLEM_WEIGHTED_SCORE_CHANGED +from opaque_keys.edx.keys import CourseKey, UsageKey +from xblock.completable import XBlockCompletionMode +from xblock.core import XBlock from .models import BlockCompletion from . import waffle @@ -21,9 +23,14 @@ def scorable_block_completion(sender, **kwargs): # pylint: disable=unused-argum """ if not waffle.waffle().is_enabled(waffle.ENABLE_COMPLETION_TRACKING): return - user = User.objects.get(id=kwargs['user_id']) course_key = CourseKey.from_string(kwargs['course_id']) block_key = UsageKey.from_string(kwargs['usage_id']) + block_cls = XBlock.load_class(block_key.block_type) + if getattr(block_cls, 'completion_mode', XBlockCompletionMode.COMPLETABLE) != XBlockCompletionMode.COMPLETABLE: + return + if getattr(block_cls, 'has_custom_completion', False): + return + user = User.objects.get(id=kwargs['user_id']) if kwargs.get('score_deleted'): completion = 0.0 else: diff --git a/lms/djangoapps/completion/tests/test_handlers.py b/lms/djangoapps/completion/tests/test_handlers.py index 813a068392..21330d8a1f 100644 --- a/lms/djangoapps/completion/tests/test_handlers.py +++ b/lms/djangoapps/completion/tests/test_handlers.py @@ -10,6 +10,8 @@ from mock import patch from opaque_keys.edx.keys import CourseKey from pytz import utc import six +from xblock.completable import XBlockCompletionMode +from xblock.core import XBlock from lms.djangoapps.grades.signals.signals import PROBLEM_WEIGHTED_SCORE_CHANGED from student.tests.factories import UserFactory @@ -19,6 +21,24 @@ from ..models import BlockCompletion from ..test_utils import CompletionWaffleTestMixin +class CustomScorableBlock(XBlock): + """ + A scorable block with a custom completion strategy. + """ + has_score = True + has_custom_completion = True + completion_mode = XBlockCompletionMode.COMPLETABLE + + +class ExcludedScorableBlock(XBlock): + """ + A scorable block that is excluded from completion tracking. + """ + has_score = True + has_custom_completion = False + completion_mode = XBlockCompletionMode.EXCLUDED + + @ddt.ddt class ScorableCompletionHandlerTestCase(CompletionWaffleTestMixin, TestCase): """ @@ -27,43 +47,79 @@ class ScorableCompletionHandlerTestCase(CompletionWaffleTestMixin, TestCase): def setUp(self): super(ScorableCompletionHandlerTestCase, self).setUp() + self.course_key = CourseKey.from_string('edx/course/beta') + self.scorable_block_key = self.course_key.make_usage_key(block_type='problem', block_id='red') self.user = UserFactory.create() - self.course_key = CourseKey.from_string("course-v1:a+valid+course") - self.block_key = self.course_key.make_usage_key(block_type="video", block_id="mah-video") self.override_waffle_switch(True) - @ddt.data( - ({'score_deleted': True}, 0.0), - ({'score_deleted': False}, 1.0), - ({}, 1.0), - ) - @ddt.unpack - def test_handler_submits_completion(self, params, expected_completion): + def call_scorable_block_completion_handler(self, block_key, score_deleted=None): + """ + Call the scorable completion signal handler for the specified block. + + Optionally takes a value to pass as score_deleted. + """ + if score_deleted is None: + params = {} + else: + params = {'score_deleted': score_deleted} handlers.scorable_block_completion( sender=self, user_id=self.user.id, course_id=six.text_type(self.course_key), - usage_id=six.text_type(self.block_key), + usage_id=six.text_type(block_key), weighted_earned=0.0, weighted_possible=3.0, modified=datetime.utcnow().replace(tzinfo=utc), score_db_table='submissions', **params ) - completion = BlockCompletion.objects.get(user=self.user, course_key=self.course_key, block_key=self.block_key) + + @ddt.data( + (True, 0.0), + (False, 1.0), + (None, 1.0), + ) + @ddt.unpack + def test_handler_submits_completion(self, score_deleted, expected_completion): + self.call_scorable_block_completion_handler(self.scorable_block_key, score_deleted) + completion = BlockCompletion.objects.get( + user=self.user, + course_key=self.course_key, + block_key=self.scorable_block_key, + ) self.assertEqual(completion.completion, expected_completion) + @XBlock.register_temp_plugin(CustomScorableBlock, 'custom_scorable') + def test_handler_skips_custom_block(self): + custom_block_key = self.course_key.make_usage_key(block_type='custom_scorable', block_id='green') + self.call_scorable_block_completion_handler(custom_block_key) + completion = BlockCompletion.objects.filter( + user=self.user, + course_key=self.course_key, + block_key=custom_block_key, + ) + self.assertFalse(completion.exists()) + + @XBlock.register_temp_plugin(ExcludedScorableBlock, 'excluded_scorable') + def test_handler_skips_excluded_block(self): + excluded_block_key = self.course_key.make_usage_key(block_type='excluded_scorable', block_id='blue') + self.call_scorable_block_completion_handler(excluded_block_key) + completion = BlockCompletion.objects.filter( + user=self.user, + course_key=self.course_key, + block_key=excluded_block_key, + ) + self.assertFalse(completion.exists()) + def test_signal_calls_handler(self): user = UserFactory.create() - course_key = CourseKey.from_string("course-v1:a+valid+course") - block_key = course_key.make_usage_key(block_type="video", block_id="mah-video") with patch('lms.djangoapps.completion.handlers.scorable_block_completion') as mock_handler: PROBLEM_WEIGHTED_SCORE_CHANGED.send_robust( sender=self, user_id=user.id, - course_id=six.text_type(course_key), - usage_id=six.text_type(block_key), + course_id=six.text_type(self.course_key), + usage_id=six.text_type(self.scorable_block_key), weighted_earned=0.0, weighted_possible=3.0, modified=datetime.utcnow().replace(tzinfo=utc),