Merge pull request #28009 from edx/ddumesnil/special-exam-completeness-aa-869
feat: AA-869: Add in logic to mark special exams as complete
This commit is contained in:
@@ -12,10 +12,11 @@ from opaque_keys.edx.keys import CourseKey, UsageKey
|
||||
|
||||
import lms.djangoapps.instructor.enrollment as enrollment
|
||||
from common.djangoapps.student import auth
|
||||
from common.djangoapps.student.models import get_user_by_username_or_email
|
||||
from common.djangoapps.student.roles import CourseStaffRole
|
||||
from lms.djangoapps.commerce.utils import create_zendesk_ticket
|
||||
from lms.djangoapps.courseware.models import StudentModule
|
||||
from lms.djangoapps.instructor.views.tools import get_student_from_identifier
|
||||
from lms.djangoapps.instructor.tasks import complete_student_attempt_task
|
||||
from xmodule.modulestore.django import modulestore
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
@@ -44,15 +45,12 @@ class InstructorService:
|
||||
course_id = CourseKey.from_string(course_id)
|
||||
|
||||
try:
|
||||
student = get_student_from_identifier(student_identifier)
|
||||
student = get_user_by_username_or_email(student_identifier)
|
||||
except ObjectDoesNotExist:
|
||||
err_msg = (
|
||||
'Error occurred while attempting to reset student attempts for user '
|
||||
'{student_identifier} for content_id {content_id}. '
|
||||
'User does not exist!'.format(
|
||||
student_identifier=student_identifier,
|
||||
content_id=content_id
|
||||
)
|
||||
f'{student_identifier} for content_id {content_id}. '
|
||||
'User does not exist!'
|
||||
)
|
||||
log.error(err_msg)
|
||||
return
|
||||
@@ -78,13 +76,26 @@ class InstructorService:
|
||||
except (StudentModule.DoesNotExist, enrollment.sub_api.SubmissionError):
|
||||
err_msg = (
|
||||
'Error occurred while attempting to reset student attempts for user '
|
||||
'{student_identifier} for content_id {content_id}.'.format(
|
||||
student_identifier=student_identifier,
|
||||
content_id=content_id
|
||||
)
|
||||
f'{student_identifier} for content_id {content_id}.'
|
||||
)
|
||||
log.error(err_msg)
|
||||
|
||||
def complete_student_attempt(self, user_identifier: str, content_id: str) -> None:
|
||||
"""
|
||||
Calls the complete_student_attempt_task
|
||||
|
||||
The task submits all completable xblocks inside of the content_id block to the
|
||||
Completion Service to mark them as complete. One use case of this function is
|
||||
for special exams (timed/proctored) where regardless of submission status on
|
||||
individual problems, we want to mark the entire exam as complete when the exam
|
||||
is finished.
|
||||
|
||||
params:
|
||||
user_identifier (str): username or email of a user
|
||||
content_id (str): the block key for a piece of content
|
||||
"""
|
||||
complete_student_attempt_task.apply_async((user_identifier, content_id))
|
||||
|
||||
def is_course_staff(self, user, course_id):
|
||||
"""
|
||||
Returns True if the user is the course staff
|
||||
|
||||
99
lms/djangoapps/instructor/tasks.py
Normal file
99
lms/djangoapps/instructor/tasks.py
Normal file
@@ -0,0 +1,99 @@
|
||||
""" Celery Tasks for the Instructor App """
|
||||
|
||||
import logging
|
||||
|
||||
from celery import shared_task
|
||||
from celery_utils.logged_task import LoggedTask
|
||||
from django.core.exceptions import ObjectDoesNotExist
|
||||
from opaque_keys import InvalidKeyError
|
||||
from opaque_keys.edx.keys import UsageKey
|
||||
from xblock.completable import XBlockCompletionMode
|
||||
|
||||
from common.djangoapps.student.models import get_user_by_username_or_email
|
||||
from lms.djangoapps.courseware.model_data import FieldDataCache
|
||||
from lms.djangoapps.courseware.module_render import get_module_for_descriptor
|
||||
from openedx.core.lib.request_utils import get_request_or_stub
|
||||
from xmodule.modulestore.django import modulestore
|
||||
from xmodule.modulestore.exceptions import ItemNotFoundError
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
|
||||
@shared_task(base=LoggedTask, ignore_result=True)
|
||||
def complete_student_attempt_task(user_identifier: str, content_id: str) -> None:
|
||||
"""
|
||||
Marks all completable children of content_id as complete for the user
|
||||
|
||||
Submits all completable xblocks inside of the content_id block to the
|
||||
Completion Service to mark them as complete. One use case of this function is
|
||||
for special exams (timed/proctored) where regardless of submission status on
|
||||
individual problems, we want to mark the entire exam as complete when the exam
|
||||
is finished.
|
||||
|
||||
params:
|
||||
user_identifier (str): username or email of a user
|
||||
content_id (str): the block key for a piece of content
|
||||
"""
|
||||
err_msg_prefix = (
|
||||
'Error occurred while attempting to complete student attempt for user '
|
||||
f'{user_identifier} for content_id {content_id}. '
|
||||
)
|
||||
err_msg = None
|
||||
try:
|
||||
user = get_user_by_username_or_email(user_identifier)
|
||||
block_key = UsageKey.from_string(content_id)
|
||||
root_descriptor = modulestore().get_item(block_key)
|
||||
except ObjectDoesNotExist:
|
||||
err_msg = err_msg_prefix + 'User does not exist!'
|
||||
except InvalidKeyError:
|
||||
err_msg = err_msg_prefix + 'Invalid content_id!'
|
||||
except ItemNotFoundError:
|
||||
err_msg = err_msg_prefix + 'Block not found in the modulestore!'
|
||||
if err_msg:
|
||||
log.error(err_msg)
|
||||
return
|
||||
|
||||
# This logic has been copied over from openedx/core/djangoapps/schedules/content_highlights.py
|
||||
# in the _get_course_module function.
|
||||
# I'm not sure if this is an anti-pattern or not, so if you can avoid re-copying this, please do.
|
||||
# We are using it here because we ran into issues with the User service being undefined when we
|
||||
# encountered a split_test xblock.
|
||||
|
||||
# Fake a request to fool parts of the courseware that want to inspect it.
|
||||
request = get_request_or_stub()
|
||||
request.user = user
|
||||
|
||||
# Now evil modulestore magic to inflate our descriptor with user state and
|
||||
# permissions checks.
|
||||
field_data_cache = FieldDataCache.cache_for_descriptor_descendents(
|
||||
root_descriptor.course_id, user, root_descriptor, read_only=True,
|
||||
)
|
||||
root_module = get_module_for_descriptor(
|
||||
user, request, root_descriptor, field_data_cache, root_descriptor.course_id,
|
||||
)
|
||||
if not root_module:
|
||||
err_msg = err_msg_prefix + 'Module unable to be created from descriptor!'
|
||||
log.error(err_msg)
|
||||
return
|
||||
|
||||
def _submit_completions(block, user):
|
||||
"""
|
||||
Recursively submits the children for completion to the Completion Service
|
||||
"""
|
||||
mode = XBlockCompletionMode.get_mode(block)
|
||||
if mode == XBlockCompletionMode.COMPLETABLE:
|
||||
block.runtime.publish(block, 'completion', {'completion': 1.0, 'user_id': user.id})
|
||||
elif mode == XBlockCompletionMode.AGGREGATOR:
|
||||
# I know this looks weird, but at the time of writing at least, there isn't a good
|
||||
# single way to get the children assigned for a partcular user. Some blocks define the
|
||||
# child descriptors method, but others don't and with blocks like Randomized Content
|
||||
# (Library Content), the get_children method returns all children and not just assigned
|
||||
# children. So this is our way around situations like that. See also Split Test Module
|
||||
# for another use case where user state has to be taken into account via get_child_descriptors
|
||||
block_children = ((hasattr(block, 'get_child_descriptors') and block.get_child_descriptors())
|
||||
or (hasattr(block, 'get_children') and block.get_children())
|
||||
or [])
|
||||
for child in block_children:
|
||||
_submit_completions(child, user)
|
||||
|
||||
_submit_completions(root_module, user)
|
||||
@@ -5,7 +5,9 @@ import json
|
||||
from unittest import mock
|
||||
|
||||
import pytest
|
||||
from completion.waffle import ENABLE_COMPLETION_TRACKING_SWITCH
|
||||
from django.core.exceptions import ObjectDoesNotExist
|
||||
from edx_toggles.toggles.testutils import override_waffle_switch
|
||||
from opaque_keys import InvalidKeyError
|
||||
|
||||
from common.djangoapps.student.models import CourseEnrollment
|
||||
@@ -15,7 +17,8 @@ from lms.djangoapps.instructor.access import allow_access
|
||||
from lms.djangoapps.instructor.services import InstructorService
|
||||
from lms.djangoapps.instructor.tests.test_tools import msk_from_problem_urlname
|
||||
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
|
||||
from xmodule.modulestore.tests.factories import CourseFactory
|
||||
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
|
||||
from xmodule.partitions.partitions import Group, UserPartition
|
||||
|
||||
|
||||
class InstructorServiceTests(SharedModuleStoreTestCase):
|
||||
@@ -38,6 +41,8 @@ class InstructorServiceTests(SharedModuleStoreTestCase):
|
||||
)
|
||||
cls.problem_urlname = str(cls.problem_location)
|
||||
cls.other_problem_urlname = str(cls.other_problem_location)
|
||||
cls.complete_error_prefix = ('Error occurred while attempting to complete student attempt for '
|
||||
'user {user} for content_id {content_id}. ')
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
@@ -113,6 +118,128 @@ class InstructorServiceTests(SharedModuleStoreTestCase):
|
||||
)
|
||||
assert result is None
|
||||
|
||||
@mock.patch('completion.handlers.BlockCompletion.objects.submit_completion')
|
||||
def test_complete_student_attempt_success(self, mock_submit):
|
||||
"""
|
||||
Assert complete_student_attempt correctly publishes completion for all
|
||||
completable children of the given content_id
|
||||
"""
|
||||
# Section, subsection, and unit are all aggregators and not completable so should
|
||||
# not be submitted.
|
||||
section = ItemFactory.create(parent=self.course, category='chapter')
|
||||
subsection = ItemFactory.create(parent=section, category='sequential')
|
||||
unit = ItemFactory.create(parent=subsection, category='vertical')
|
||||
|
||||
# should both be submitted
|
||||
video = ItemFactory.create(parent=unit, category='video')
|
||||
problem = ItemFactory.create(parent=unit, category='problem')
|
||||
|
||||
# Not a completable block
|
||||
ItemFactory.create(parent=unit, category='discussion')
|
||||
|
||||
with override_waffle_switch(ENABLE_COMPLETION_TRACKING_SWITCH, True):
|
||||
self.service.complete_student_attempt(self.student.username, str(subsection.location))
|
||||
|
||||
# Only Completable leaf blocks should have completion published
|
||||
assert mock_submit.call_count == 2
|
||||
mock_submit.assert_any_call(user=self.student, block_key=video.location, completion=1.0)
|
||||
mock_submit.assert_any_call(user=self.student, block_key=problem.location, completion=1.0)
|
||||
|
||||
@mock.patch('completion.handlers.BlockCompletion.objects.submit_completion')
|
||||
def test_complete_student_attempt_split_test(self, mock_submit):
|
||||
"""
|
||||
Asserts complete_student_attempt correctly publishes completion when a split test is involved
|
||||
|
||||
This test case exists because we ran into a bug about the user_service not existing
|
||||
when a split_test existed inside of a subsection. Associated with this change was adding
|
||||
in the user state into the module before attempting completion and this ensures that is
|
||||
working properly.
|
||||
"""
|
||||
partition = UserPartition(
|
||||
0,
|
||||
'first_partition',
|
||||
'First Partition',
|
||||
[
|
||||
Group(0, 'alpha'),
|
||||
Group(1, 'beta')
|
||||
]
|
||||
)
|
||||
course = CourseFactory.create(user_partitions=[partition])
|
||||
section = ItemFactory.create(parent=course, category='chapter')
|
||||
subsection = ItemFactory.create(parent=section, category='sequential')
|
||||
|
||||
c0_url = course.id.make_usage_key('vertical', 'split_test_cond0')
|
||||
c1_url = course.id.make_usage_key('vertical', 'split_test_cond1')
|
||||
split_test = ItemFactory.create(
|
||||
parent=subsection,
|
||||
category='split_test',
|
||||
user_partition_id=0,
|
||||
group_id_to_child={'0': c0_url, '1': c1_url},
|
||||
)
|
||||
|
||||
cond0vert = ItemFactory.create(parent=split_test, category='vertical', location=c0_url)
|
||||
ItemFactory.create(parent=cond0vert, category='video')
|
||||
ItemFactory.create(parent=cond0vert, category='problem')
|
||||
|
||||
cond1vert = ItemFactory.create(parent=split_test, category='vertical', location=c1_url)
|
||||
ItemFactory.create(parent=cond1vert, category='video')
|
||||
ItemFactory.create(parent=cond1vert, category='html')
|
||||
|
||||
with override_waffle_switch(ENABLE_COMPLETION_TRACKING_SWITCH, True):
|
||||
self.service.complete_student_attempt(self.student.username, str(subsection.location))
|
||||
|
||||
# Only the group the user was assigned to should have completion published.
|
||||
# Either cond0vert's children or cond1vert's children
|
||||
assert mock_submit.call_count == 2
|
||||
|
||||
@mock.patch('lms.djangoapps.instructor.tasks.log.error')
|
||||
def test_complete_student_attempt_bad_user(self, mock_logger):
|
||||
"""
|
||||
Assert complete_student_attempt with a bad user raises error and returns None
|
||||
"""
|
||||
username = 'bad_user'
|
||||
self.service.complete_student_attempt(username, self.problem_urlname)
|
||||
mock_logger.assert_called_once_with(
|
||||
self.complete_error_prefix.format(user=username, content_id=self.problem_urlname) + 'User does not exist!'
|
||||
)
|
||||
|
||||
@mock.patch('lms.djangoapps.instructor.tasks.log.error')
|
||||
def test_complete_student_attempt_bad_content_id(self, mock_logger):
|
||||
"""
|
||||
Assert complete_student_attempt with a bad content_id raises error and returns None
|
||||
"""
|
||||
username = self.student.username
|
||||
self.service.complete_student_attempt(username, 'foo/bar/baz')
|
||||
mock_logger.assert_called_once_with(
|
||||
self.complete_error_prefix.format(user=username, content_id='foo/bar/baz') + 'Invalid content_id!'
|
||||
)
|
||||
|
||||
@mock.patch('lms.djangoapps.instructor.tasks.log.error')
|
||||
def test_complete_student_attempt_nonexisting_item(self, mock_logger):
|
||||
"""
|
||||
Assert complete_student_attempt with nonexisting item in the modulestore
|
||||
raises error and returns None
|
||||
"""
|
||||
username = self.student.username
|
||||
block = self.problem_urlname
|
||||
self.service.complete_student_attempt(username, block)
|
||||
mock_logger.assert_called_once_with(
|
||||
self.complete_error_prefix.format(user=username, content_id=block) + 'Block not found in the modulestore!'
|
||||
)
|
||||
|
||||
@mock.patch('lms.djangoapps.instructor.tasks.log.error')
|
||||
def test_complete_student_attempt_failed_module(self, mock_logger):
|
||||
"""
|
||||
Assert complete_student_attempt with failed get_module raises error and returns None
|
||||
"""
|
||||
username = self.student.username
|
||||
with mock.patch('lms.djangoapps.instructor.tasks.get_module_for_descriptor', return_value=None):
|
||||
self.service.complete_student_attempt(username, str(self.course.location))
|
||||
mock_logger.assert_called_once_with(
|
||||
self.complete_error_prefix.format(user=username, content_id=self.course.location) +
|
||||
'Module unable to be created from descriptor!'
|
||||
)
|
||||
|
||||
def test_is_user_staff(self):
|
||||
"""
|
||||
Test to assert that the user is staff or not
|
||||
|
||||
@@ -86,9 +86,6 @@ social-auth-core<4.0.0 # social-auth-core>=4.0.0 requires PYJWT>=2.0.0
|
||||
# celery requires click<8.0.0 which would be fixed once https://github.com/celery/celery/issues/6753 is done.
|
||||
click<8.0.0
|
||||
|
||||
# https://openedx.atlassian.net/servicedesk/customer/portal/9/CR-3776
|
||||
edx-proctoring==3.13.2
|
||||
|
||||
# constraints present due to Python35 support. Need to be tested and removed independently.
|
||||
|
||||
celery<5.0 # celery 5.0 has dropped python3.5 support.
|
||||
|
||||
@@ -454,9 +454,8 @@ edx-opaque-keys[django]==2.2.1
|
||||
# xmodule
|
||||
edx-organizations==6.9.0
|
||||
# via -r requirements/edx/base.in
|
||||
edx-proctoring==3.13.2
|
||||
edx-proctoring==3.14.0
|
||||
# via
|
||||
# -c requirements/edx/../constraints.txt
|
||||
# -r requirements/edx/base.in
|
||||
# edx-proctoring-proctortrack
|
||||
edx-proctoring-proctortrack==1.0.5
|
||||
|
||||
@@ -542,9 +542,8 @@ edx-opaque-keys[django]==2.2.1
|
||||
# xmodule
|
||||
edx-organizations==6.9.0
|
||||
# via -r requirements/edx/testing.txt
|
||||
edx-proctoring==3.13.2
|
||||
edx-proctoring==3.14.0
|
||||
# via
|
||||
# -c requirements/edx/../constraints.txt
|
||||
# -r requirements/edx/testing.txt
|
||||
# edx-proctoring-proctortrack
|
||||
edx-proctoring-proctortrack==1.0.5
|
||||
|
||||
@@ -528,9 +528,8 @@ edx-opaque-keys[django]==2.2.1
|
||||
# xmodule
|
||||
edx-organizations==6.9.0
|
||||
# via -r requirements/edx/base.txt
|
||||
edx-proctoring==3.13.2
|
||||
edx-proctoring==3.14.0
|
||||
# via
|
||||
# -c requirements/edx/../constraints.txt
|
||||
# -r requirements/edx/base.txt
|
||||
# edx-proctoring-proctortrack
|
||||
edx-proctoring-proctortrack==1.0.5
|
||||
|
||||
Reference in New Issue
Block a user