diff --git a/lms/djangoapps/completion/models.py b/lms/djangoapps/completion/models.py index 9561f6fb6d..528e20224e 100644 --- a/lms/djangoapps/completion/models.py +++ b/lms/djangoapps/completion/models.py @@ -12,6 +12,7 @@ from model_utils.models import TimeStampedModel from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.xmodule_django.models import CourseKeyField, UsageKeyField +from . import waffle # pylint: disable=ungrouped-imports try: @@ -52,7 +53,8 @@ class BlockCompletionManager(models.Manager): Return Value: (BlockCompletion, bool): A tuple comprising the created or updated - BlockCompletion object and a boolean value indicating whether the value + BlockCompletion object and a boolean value indicating whether the + object was newly created by this call. Raises: @@ -84,17 +86,23 @@ class BlockCompletionManager(models.Manager): "block_key must be an instance of `opaque_keys.edx.keys.UsageKey`. Got {}".format(type(block_key)) ) - obj, isnew = self.get_or_create( - user=user, - course_key=course_key, - block_type=block_type, - block_key=block_key, - defaults={'completion': completion}, - ) - if not isnew and obj.completion != completion: - obj.completion = completion - obj.full_clean() - obj.save() + if waffle.waffle().is_enabled(waffle.ENABLE_COMPLETION_TRACKING): + obj, isnew = self.get_or_create( + user=user, + course_key=course_key, + block_type=block_type, + block_key=block_key, + defaults={'completion': completion}, + ) + if not isnew and obj.completion != completion: + obj.completion = completion + obj.full_clean() + obj.save() + else: + # If the feature is not enabled, this method should not be called. Error out with a RuntimeError. + raise RuntimeError( + "BlockCompletion.objects.submit_completion should not be called when the feature is disabled." + ) return obj, isnew diff --git a/lms/djangoapps/completion/tests/test_models.py b/lms/djangoapps/completion/tests/test_models.py index 6bc61010c3..c38341dddf 100644 --- a/lms/djangoapps/completion/tests/test_models.py +++ b/lms/djangoapps/completion/tests/test_models.py @@ -2,6 +2,8 @@ Test models, managers, and validators. """ +from __future__ import absolute_import, division, print_function, unicode_literals + from django.core.exceptions import ValidationError from django.test import TestCase from opaque_keys.edx.keys import UsageKey @@ -9,6 +11,7 @@ from opaque_keys.edx.keys import UsageKey from student.tests.factories import UserFactory from .. import models +from .. import waffle class PercentValidatorTestCase(TestCase): @@ -24,13 +27,8 @@ class PercentValidatorTestCase(TestCase): self.assertRaises(ValidationError, models.validate_percent, value) -class SubmitCompletionTestCase(TestCase): - """ - Test that BlockCompletion.objects.submit_completion has the desired - semantics. - """ - def setUp(self): - super(SubmitCompletionTestCase, self).setUp() +class CompletionSetUpMixin(object): + def set_up_completion(self): self.user = UserFactory() self.block_key = UsageKey.from_string(u'block-v1:edx+test+run+type@video+block@doggos') self.completion = models.BlockCompletion.objects.create( @@ -41,6 +39,19 @@ class SubmitCompletionTestCase(TestCase): completion=0.5, ) + +class SubmitCompletionTestCase(CompletionSetUpMixin, TestCase): + """ + Test that BlockCompletion.objects.submit_completion has the desired + semantics. + """ + 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) + self.set_up_completion() + def test_changed_value(self): with self.assertNumQueries(4): # Get, update, 2 * savepoints completion, isnew = models.BlockCompletion.objects.submit_completion( @@ -102,3 +113,32 @@ class SubmitCompletionTestCase(TestCase): completion = models.BlockCompletion.objects.get(user=self.user, block_key=self.block_key) self.assertEqual(completion.completion, 0.5) self.assertEqual(models.BlockCompletion.objects.count(), 1) + + +class CompletionDisabledTestCase(CompletionSetUpMixin, TestCase): + + @classmethod + def setUpClass(cls): + super(CompletionDisabledTestCase, cls).setUpClass() + cls.overrider = waffle.waffle().override(waffle.ENABLE_COMPLETION_TRACKING, False) + cls.overrider.__enter__() + + @classmethod + def tearDownClass(cls): + cls.overrider.__exit__(None, None, None) + super(CompletionDisabledTestCase, cls).tearDownClass() + + def setUp(self): + super(CompletionDisabledTestCase, self).setUp() + self.set_up_completion() + + def test_cannot_call_submit_completion(self): + self.assertEqual(models.BlockCompletion.objects.count(), 1) + with self.assertRaises(RuntimeError): + models.BlockCompletion.objects.submit_completion( + user=self.user, + course_key=self.block_key.course_key, + block_key=self.block_key, + completion=0.9, + ) + self.assertEqual(models.BlockCompletion.objects.count(), 1) diff --git a/lms/djangoapps/completion/waffle.py b/lms/djangoapps/completion/waffle.py new file mode 100644 index 0000000000..7e0fe70fff --- /dev/null +++ b/lms/djangoapps/completion/waffle.py @@ -0,0 +1,20 @@ +""" +This module contains various configuration settings via +waffle switches for the completion app. +""" +from __future__ import absolute_import, division, print_function, unicode_literals + +from openedx.core.djangoapps.waffle_utils import WaffleSwitchNamespace + +# Namespace +WAFFLE_NAMESPACE = 'completion' + +# Switches +ENABLE_COMPLETION_TRACKING = 'enable_completion_tracking' + + +def waffle(): + """ + Returns the namespaced, cached, audited Waffle class for completion. + """ + return WaffleSwitchNamespace(name=WAFFLE_NAMESPACE, log_prefix='completion: ') diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index c4f7a01dfb..d782f4eb25 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -39,6 +39,8 @@ from courseware.masquerade import ( from courseware.model_data import DjangoKeyValueStore, FieldDataCache from edxmako.shortcuts import render_to_string from eventtracking import tracker +from lms.djangoapps.completion.models import BlockCompletion +from lms.djangoapps.completion import waffle as completion_waffle from lms.djangoapps.grades.signals.signals import SCORE_PUBLISHED from lms.djangoapps.lms_xblock.field_data import LmsFieldData from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig @@ -384,12 +386,23 @@ def get_module_for_descriptor(user, request, descriptor, field_data_cache, cours ) -def get_module_system_for_user(user, student_data, # TODO # pylint: disable=too-many-statements - # Arguments preceding this comment have user binding, those following don't - descriptor, course_id, track_function, xqueue_callback_url_prefix, - request_token, position=None, wrap_xmodule_display=True, grade_bucket_type=None, - static_asset_path='', user_location=None, disable_staff_debug_info=False, - course=None): +def get_module_system_for_user( + user, + student_data, # TODO # pylint: disable=too-many-statements + # Arguments preceding this comment have user binding, those following don't + descriptor, + course_id, + track_function, + xqueue_callback_url_prefix, + request_token, + position=None, + wrap_xmodule_display=True, + grade_bucket_type=None, + static_asset_path='', + user_location=None, + disable_staff_debug_info=False, + course=None +): """ Helper function that returns a module system and student_data bound to a user and a descriptor. @@ -461,18 +474,26 @@ def get_module_system_for_user(user, student_data, # TODO # pylint: disable=to course=course ) + def get_event_handler(event_type): + """ + Return an appropriate function to handle the event. + + Returns None if no special processing is required. + """ + handlers = { + 'completion': handle_completion_event, + 'grade': handle_grade_event, + 'progress': handle_deprecated_progress_event, + } + return handlers.get(event_type) + def publish(block, event_type, event): - """A function that allows XModules to publish events.""" - if event_type == 'grade' and not is_masquerading_as_specific_student(user, course_id): - SCORE_PUBLISHED.send( - sender=None, - block=block, - user=user, - raw_earned=event['value'], - raw_possible=event['max_value'], - only_if_higher=event.get('only_if_higher'), - score_deleted=event.get('score_deleted'), - ) + """ + A function that allows XModules to publish events. + """ + handle_event = get_event_handler(event_type) + if handle_event and not is_masquerading_as_specific_student(user, course_id): + handle_event(block, event) else: context = contexts.course_context_from_course_id(course_id) if block.runtime.user_id: @@ -486,6 +507,57 @@ def get_module_system_for_user(user, student_data, # TODO # pylint: disable=to with tracker.get_tracker().context(event_type, context): track_function(event_type, event) + def handle_completion_event(block, event): + """ + Submit a completion object for the block. + """ + if not completion_waffle.waffle().is_enabled(completion_waffle.ENABLE_COMPLETION_TRACKING): + raise Http404 + else: + BlockCompletion.objects.submit_completion( + user=user, + course_key=course_id, + block_key=block.scope_ids.usage_id, + completion=event['completion'], + ) + + def handle_grade_event(block, event): + """ + Submit a grade for the block. + """ + SCORE_PUBLISHED.send( + sender=None, + block=block, + user=user, + raw_earned=event['value'], + raw_possible=event['max_value'], + only_if_higher=event.get('only_if_higher'), + score_deleted=event.get('score_deleted'), + ) + + def handle_deprecated_progress_event(block, event): + """ + DEPRECATED: Submit a completion for the block represented by the + progress event. + + This exists to support the legacy progress extension used by + edx-solutions. New XBlocks should not emit these events, but instead + emit completion events directly. + """ + if not completion_waffle.waffle().is_enabled(completion_waffle.ENABLE_COMPLETION_TRACKING): + raise Http404 + else: + requested_user_id = event.get('user_id', user.id) + if requested_user_id != user.id: + log.warning("{} tried to submit a completion on behalf of {}".format(user, requested_user_id)) + return + BlockCompletion.objects.submit_completion( + user=user, + course_key=course_id, + block_key=block.scope_ids.usage_id, + completion=1.0, + ) + def rebind_noauth_module_to_user(module, real_user): """ A function that allows a module to get re-bound to a real user if it was previously bound to an AnonymousUser. diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 38d644ec83..16df6f2e27 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -36,12 +36,15 @@ from course_modes.models import CourseMode from courseware import module_render as render from courseware.courses import get_course_info_section, get_course_with_access from courseware.field_overrides import OverrideFieldData +from courseware.masquerade import CourseMasquerade from courseware.model_data import FieldDataCache from courseware.models import StudentModule from courseware.module_render import get_module_for_descriptor, hash_resource from courseware.tests.factories import GlobalStaffFactory, StudentModuleFactory, UserFactory from courseware.tests.test_submitting_problems import TestSubmittingProblems from courseware.tests.tests import LoginEnrollmentTestCase +from lms.djangoapps.completion.models import BlockCompletion +from lms.djangoapps.completion import waffle as completion_waffle from lms.djangoapps.lms_xblock.field_data import LmsFieldData from openedx.core.djangoapps.credit.api import set_credit_requirement_status, set_credit_requirements from openedx.core.djangoapps.credit.models import CreditCourse @@ -113,6 +116,31 @@ class GradedStatelessXBlock(XBlock): ) +class StubCompletableXBlock(XBlock): + """ + This XBlock exists to test completion storage. + """ + + @XBlock.json_handler + def complete(self, json_data, suffix): # pylint: disable=unused-argument + """ + Mark the block's completion value using the completion API. + """ + return self.runtime.publish( + self, + 'completion', + {'completion': json_data['completion']}, + ) + + @XBlock.json_handler + 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', {}) + + @attr(shard=1) @ddt.ddt class ModuleRenderTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase): @@ -578,6 +606,120 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas self.assertEquals(student_module.grade, 0.75) self.assertEquals(student_module.max_grade, 1) + @XBlock.register_temp_plugin(StubCompletableXBlock, identifier='comp') + def test_completion_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({'completion': 0.625}), + content_type='application/json', + ) + request.user = self.mock_user + with self.assertRaises(Http404): + result = render.handle_xblock_callback( + request, + unicode(course.id), + quote_slashes(unicode(block.scope_ids.usage_id)), + 'complete', + '', + ) + + @XBlock.register_temp_plugin(StubCompletableXBlock, identifier='comp') + def test_completion_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({'completion': 0.625}), + 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)), + 'complete', + '', + ) + 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) + + @XBlock.register_temp_plugin(StubCompletableXBlock, identifier='comp') + def test_skip_handlers_for_masquerading_staff(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({'completion': 0.8}), + content_type='application/json', + ) + request.user = self.mock_user + request.session = {} + request.user.real_user = GlobalStaffFactory.create() + request.user.real_user.masquerade_settings = CourseMasquerade(course.id, user_name="jem") + with patch('courseware.module_render.is_masquerading_as_specific_student') as mock_masq: + mock_masq.return_value = True + response = render.handle_xblock_callback( + request, + unicode(course.id), + quote_slashes(unicode(block.scope_ids.usage_id)), + 'complete', + '', + ) + mock_masq.assert_called() + self.assertEqual(response.status_code, 200) + with self.assertRaises(BlockCompletion.DoesNotExist): + BlockCompletion.objects.get(block_key=block.scope_ids.usage_id) + @patch.dict('django.conf.settings.FEATURES', {'ENABLE_XBLOCK_VIEW_ENDPOINT': True}) def test_xblock_view_handler(self): args = [