Add handler to receive completion events
* Submit a completion when receiving a completion event from an XBlock. * Handle legacy progress events. * Convert handler to use a dispatch dict instead of an if-else chain. * Extract masquerade checking from individual handlers. * Gate submit_completion on waffle switch * 404 on handler views when trying to submit completion without waffle switch enabled. OC-3087 Disallow calling submit_completion when waffle flag is disabled. Add tests that trying to publish completion errors.
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
20
lms/djangoapps/completion/waffle.py
Normal file
20
lms/djangoapps/completion/waffle.py
Normal file
@@ -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: ')
|
||||
@@ -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.
|
||||
|
||||
@@ -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 = [
|
||||
|
||||
Reference in New Issue
Block a user