From 69271d043b04daedfbc43fbb11475a12bbc29627 Mon Sep 17 00:00:00 2001 From: "J. Cliff Dyer" Date: Wed, 4 Oct 2017 16:10:28 -0400 Subject: [PATCH] Handle completion of scorable blocks * Add a handler to mark a block complete when a problem is scored. * Also handle marking incomplete when user problem state is deleted. * Add score_deleted to published providing_args for PROBLEM_{RAW,WEIGHTED}_SCORE_CHANGED OC-3089 --- lms/djangoapps/completion/apps.py | 3 + lms/djangoapps/completion/handlers.py | 36 ++++++ .../completion/tests/test_handlers.py | 120 ++++++++++++++++++ .../completion/tests/test_models.py | 6 +- .../courseware/tests/test_module_render.py | 76 ++++------- lms/djangoapps/grades/signals/signals.py | 4 + 6 files changed, 188 insertions(+), 57 deletions(-) create mode 100644 lms/djangoapps/completion/handlers.py create mode 100644 lms/djangoapps/completion/tests/test_handlers.py diff --git a/lms/djangoapps/completion/apps.py b/lms/djangoapps/completion/apps.py index 69babe6b73..a684576ea5 100644 --- a/lms/djangoapps/completion/apps.py +++ b/lms/djangoapps/completion/apps.py @@ -12,3 +12,6 @@ class CompletionAppConfig(AppConfig): """ name = 'lms.djangoapps.completion' verbose_name = 'Completion' + + def ready(self): + from . import handlers # pylint: disable=unused-variable diff --git a/lms/djangoapps/completion/handlers.py b/lms/djangoapps/completion/handlers.py new file mode 100644 index 0000000000..aded416de8 --- /dev/null +++ b/lms/djangoapps/completion/handlers.py @@ -0,0 +1,36 @@ +""" +Signal handlers to trigger completion updates. +""" + +from __future__ import absolute_import, division, print_function, unicode_literals + +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 .models import BlockCompletion +from . import waffle + + +@receiver(PROBLEM_WEIGHTED_SCORE_CHANGED) +def scorable_block_completion(sender, **kwargs): # pylint: disable=unused-argument + """ + When a problem is scored, submit a new BlockCompletion for that block. + """ + 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']) + if kwargs.get('score_deleted'): + completion = 0.0 + else: + completion = 1.0 + BlockCompletion.objects.submit_completion( + user=user, + course_key=course_key, + block_key=block_key, + completion=completion, + ) diff --git a/lms/djangoapps/completion/tests/test_handlers.py b/lms/djangoapps/completion/tests/test_handlers.py new file mode 100644 index 0000000000..b9f393e99c --- /dev/null +++ b/lms/djangoapps/completion/tests/test_handlers.py @@ -0,0 +1,120 @@ +""" +Test signal handlers. +""" + +from datetime import datetime + +import ddt +from django.test import TestCase +from mock import patch +from opaque_keys.edx.keys import CourseKey +from pytz import utc +import six + +from lms.djangoapps.grades.signals.signals import PROBLEM_WEIGHTED_SCORE_CHANGED +from student.tests.factories import UserFactory + +from .. import handlers +from ..models import BlockCompletion +from .. import waffle + + +class CompletionHandlerMixin(object): + """ + Common functionality for completion handler tests. + """ + def override_waffle_switch(self, override): + """ + Override the setting of the ENABLE_COMPLETION_TRACKING waffle switch + for the course of the test. + + Parameters: + override (bool): True if tracking should be enabled. + """ + _waffle_overrider = waffle.waffle().override(waffle.ENABLE_COMPLETION_TRACKING, override) + _waffle_overrider.__enter__() + self.addCleanup(_waffle_overrider.__exit__, None, None, None) + + +@ddt.ddt +class ScorableCompletionHandlerTestCase(CompletionHandlerMixin, TestCase): + """ + Test the signal handler + """ + + def setUp(self): + super(ScorableCompletionHandlerTestCase, self).setUp() + 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): + 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), + 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) + self.assertEqual(completion.completion, expected_completion) + + 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), + weighted_earned=0.0, + weighted_possible=3.0, + modified=datetime.utcnow().replace(tzinfo=utc), + score_db_table='submissions', + ) + mock_handler.assert_called() + + +class DisabledCompletionHandlerTestCase(CompletionHandlerMixin, TestCase): + """ + Test that disabling the ENABLE_COMPLETION_TRACKING waffle switch prevents + the signal handler from submitting a completion. + """ + def setUp(self): + super(DisabledCompletionHandlerTestCase, self).setUp() + 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(False) + + def test_disabled_handler_does_not_submit_completion(self): + 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), + weighted_earned=0.0, + weighted_possible=3.0, + modified=datetime.utcnow().replace(tzinfo=utc), + score_db_table='submissions', + ) + with self.assertRaises(BlockCompletion.DoesNotExist): + BlockCompletion.objects.get( + user=self.user, + course_key=self.course_key, + block_key=self.block_key + ) diff --git a/lms/djangoapps/completion/tests/test_models.py b/lms/djangoapps/completion/tests/test_models.py index c38341dddf..1664732874 100644 --- a/lms/djangoapps/completion/tests/test_models.py +++ b/lms/djangoapps/completion/tests/test_models.py @@ -47,9 +47,9 @@ class SubmitCompletionTestCase(CompletionSetUpMixin, TestCase): """ def setUp(self): super(SubmitCompletionTestCase, self).setUp() - self._overrider = waffle.waffle().override(waffle.ENABLE_COMPLETION_TRACKING, True) - self._overrider.__enter__() - self.addCleanup(self._overrider.__exit__, None, None, None) + _overrider = waffle.waffle().override(waffle.ENABLE_COMPLETION_TRACKING, True) + _overrider.__enter__() + self.addCleanup(_overrider.__exit__, None, None, None) self.set_up_completion() def test_changed_value(self): diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 16df6f2e27..6a429a8bf6 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -136,6 +136,7 @@ class StubCompletableXBlock(XBlock): def progress(self, json_data, suffix): # pylint: disable=unused-argument """ Mark the block as complete using the deprecated progress interface. + New code should use the completion event instead. """ return self.runtime.publish(self, 'progress', {}) @@ -425,6 +426,7 @@ class ModuleRenderTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase): @attr(shard=1) +@ddt.ddt class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCase): """ Test the handle_xblock_callback function @@ -606,34 +608,44 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas self.assertEquals(student_module.grade, 0.75) self.assertEquals(student_module.max_grade, 1) + @ddt.data( + ('complete', {'completion': 0.625}), + ('progress', {}), + ) + @ddt.unpack @XBlock.register_temp_plugin(StubCompletableXBlock, identifier='comp') - def test_completion_event_with_completion_disabled(self): + def test_completion_events_with_completion_disabled(self, signal, data): with completion_waffle.waffle().override(completion_waffle.ENABLE_COMPLETION_TRACKING, False): course = CourseFactory.create() block = ItemFactory.create(category='comp', parent=course) request = self.request_factory.post( '/', - data=json.dumps({'completion': 0.625}), + data=json.dumps(data), content_type='application/json', ) request.user = self.mock_user with self.assertRaises(Http404): - result = render.handle_xblock_callback( + render.handle_xblock_callback( request, unicode(course.id), quote_slashes(unicode(block.scope_ids.usage_id)), - 'complete', + signal, '', ) + @ddt.data( + ('complete', {'completion': 0.625}, 0.625), + ('progress', {}, 1.0), + ) + @ddt.unpack @XBlock.register_temp_plugin(StubCompletableXBlock, identifier='comp') - def test_completion_event(self): + def test_completion_events(self, signal, data, expected_completion): with completion_waffle.waffle().override(completion_waffle.ENABLE_COMPLETION_TRACKING, True): course = CourseFactory.create() block = ItemFactory.create(category='comp', parent=course) request = self.request_factory.post( '/', - data=json.dumps({'completion': 0.625}), + data=json.dumps(data), content_type='application/json', ) request.user = self.mock_user @@ -641,56 +653,12 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas request, unicode(course.id), quote_slashes(unicode(block.scope_ids.usage_id)), - 'complete', + signal, '', ) - self.assertEqual(response.status_code, 200) - completion = BlockCompletion.objects.get(block_key=block.scope_ids.usage_id) - self.assertEqual(completion.completion, 0.625) - - @XBlock.register_temp_plugin(StubCompletableXBlock, identifier='comp') - def test_progress_event_with_completion_disabled(self): - with completion_waffle.waffle().override(completion_waffle.ENABLE_COMPLETION_TRACKING, False): - course = CourseFactory.create() - block = ItemFactory.create(category='comp', parent=course) - request = self.request_factory.post( - '/', - data=json.dumps({}), - content_type='application/json', - ) - request.user = self.mock_user - with self.assertRaises(Http404): - response = render.handle_xblock_callback( - request, - unicode(course.id), - quote_slashes(unicode(block.scope_ids.usage_id)), - 'progress', - '', - ) - self.assertEqual(response.status_code, 404) - raise Http404 - - @XBlock.register_temp_plugin(StubCompletableXBlock, identifier='comp') - def test_progress_event(self): - with completion_waffle.waffle().override(completion_waffle.ENABLE_COMPLETION_TRACKING, True): - course = CourseFactory.create() - block = ItemFactory.create(category='comp', parent=course) - request = self.request_factory.post( - '/', - data=json.dumps({}), - content_type='application/json', - ) - request.user = self.mock_user - response = render.handle_xblock_callback( - request, - unicode(course.id), - quote_slashes(unicode(block.scope_ids.usage_id)), - 'progress', - '', - ) - self.assertEqual(response.status_code, 200) - completion = BlockCompletion.objects.get(block_key=block.scope_ids.usage_id) - self.assertEqual(completion.completion, 1.0) + self.assertEqual(response.status_code, 200) + completion = BlockCompletion.objects.get(block_key=block.scope_ids.usage_id) + self.assertEqual(completion.completion, expected_completion) @XBlock.register_temp_plugin(StubCompletableXBlock, identifier='comp') def test_skip_handlers_for_masquerading_staff(self): diff --git a/lms/djangoapps/grades/signals/signals.py b/lms/djangoapps/grades/signals/signals.py index a0c34b8756..d363ed8630 100644 --- a/lms/djangoapps/grades/signals/signals.py +++ b/lms/djangoapps/grades/signals/signals.py @@ -26,6 +26,8 @@ PROBLEM_RAW_SCORE_CHANGED = Signal( 'modified', # A datetime indicating when the database representation of # this the problem score was saved. 'score_db_table', # The database table that houses the score that changed. + 'score_deleted', # Boolean indicating whether the score changed due to + # the user state being deleted. ] ) @@ -49,6 +51,8 @@ PROBLEM_WEIGHTED_SCORE_CHANGED = Signal( 'modified', # A datetime indicating when the database representation of # this the problem score was saved. 'score_db_table', # The database table that houses the score that changed. + 'score_deleted', # Boolean indicating whether the score changed due to + # the user state being deleted. ] )