Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "feat: AA-773: Add in logic to mark special exams as complete"

Browse Source
This commit is contained in:
Dillon Dumesnil
2021-06-16 10:12:33 -07:00
committed by Dillon Dumesnil
parent 09e456bce5
commit 68cce7dbbb
6 changed files with 18 additions and 142 deletions
Download Patch File Download Diff File Expand all files Collapse all files

69
lms/djangoapps/instructor/services.py
View File

@@ -9,16 +9,14 @@ from django.core.exceptions import ObjectDoesNotExist
from django.utils.translation import ugettext as _
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey, UsageKey
from xblock.completable import XBlockCompletionMode
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 xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError
log = logging.getLogger(__name__)
@@ -46,12 +44,15 @@ class InstructorService:
course_id = CourseKey.from_string(course_id)
try:
student = get_user_by_username_or_email(student_identifier)
student = get_student_from_identifier(student_identifier)
except ObjectDoesNotExist:
err_msg = (
'Error occurred while attempting to reset student attempts for user '
f'{student_identifier} for content_id {content_id}. '
'User does not exist!'
'{student_identifier} for content_id {content_id}. '
'User does not exist!'.format(
student_identifier=student_identifier,
content_id=content_id
)
)
log.error(err_msg)
return
@@ -77,61 +78,13 @@ class InstructorService:
except (StudentModule.DoesNotExist, enrollment.sub_api.SubmissionError):
err_msg = (
'Error occurred while attempting to reset student attempts for user '
f'{student_identifier} for content_id {content_id}.'
'{student_identifier} for content_id {content_id}.'.format(
student_identifier=student_identifier,
content_id=content_id
)
)
log.error(err_msg)
def complete_student_attempt(self, user_identifier, content_id):
"""
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 (String): username or email of a user
content_id (String): 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_block = 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
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.
block_children = ((hasattr(block, 'get_child_descriptors') and block.get_child_descriptors())
or (hasattr(block, 'get_children') and block.get_children()))
for child in block_children:
_submit_completions(child, user)
_submit_completions(root_block, user)
def is_course_staff(self, user, course_id):
"""
Returns True if the user is the course staff

82
lms/djangoapps/instructor/tests/test_services.py
View File

@@ -15,7 +15,7 @@ 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, ItemFactory
from xmodule.modulestore.tests.factories import CourseFactory
class InstructorServiceTests(SharedModuleStoreTestCase):
@@ -38,8 +38,6 @@ 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()
@@ -115,84 +113,6 @@ class InstructorServiceTests(SharedModuleStoreTestCase):
)
assert result is None
# So technically, this mock publish is different than what is hit when running this code
# in production (lms.djangoapps.courseware.module_render.get_module_system_for_user.publish).
# I tried to figure out why or a way to force it to be more production like and was unsuccessful,
# so if anyone figures it out at any point, it would be greatly appreciated if you could update this.
# I thought it was acceptable because I'm still able to confirm correct behavior of the function
# and the attempted call, it is just going to the wrong publish spot ¯\_(ツ)_/¯
@mock.patch('xmodule.x_module.DescriptorSystem.publish')
def test_complete_student_attempt_success(self, mock_publish):
"""
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')
self.service.complete_student_attempt(self.student.username, str(subsection.location))
# Only Completable leaf blocks should have completion published
assert mock_publish.call_count == 2
# The calls take the form of (xblock, publish_event, publish_data), which in our case
# will look like (xblock, 'completion', {'completion': 1.0, 'user_id': 1}).
# I'd prefer to be able to just assert all fields at once, but for some reason the value
# of the xblock in the mock call and the object above are different (even with a refetch)
# so I'm settling for just ensuring the location (block_key) of both are identical
video_call = mock_publish.call_args_list[0][0]
assert video_call[0].location == video.location
assert video_call[1] == 'completion'
assert video_call[2] == {'completion': 1.0, 'user_id': self.student.id}
problem_call = mock_publish.call_args_list[1][0]
assert problem_call[0].location == problem.location
assert problem_call[1] == 'completion'
assert problem_call[2] == {'completion': 1.0, 'user_id': self.student.id}
@mock.patch('lms.djangoapps.instructor.services.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.services.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.services.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!'
)
def test_is_user_staff(self):
"""
Test to assert that the user is staff or not

3
requirements/constraints.txt
View File

@@ -86,6 +86,9 @@ 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.

2
requirements/edx/base.txt
View File

@@ -457,7 +457,7 @@ edx-organizations==6.9.0
# via -r requirements/edx/base.in
edx-proctoring-proctortrack==1.0.5
# via -r requirements/edx/base.in
edx-proctoring==3.14.0
edx-proctoring==3.13.2
# via
# -r requirements/edx/base.in
# edx-proctoring-proctortrack

2
requirements/edx/development.txt
View File

@@ -547,7 +547,7 @@ edx-organizations==6.9.0
# via -r requirements/edx/testing.txt
edx-proctoring-proctortrack==1.0.5
# via -r requirements/edx/testing.txt
edx-proctoring==3.14.0
edx-proctoring==3.13.2
# via
# -r requirements/edx/testing.txt
# edx-proctoring-proctortrack

2
requirements/edx/testing.txt
View File

@@ -533,7 +533,7 @@ edx-organizations==6.9.0
# via -r requirements/edx/base.txt
edx-proctoring-proctortrack==1.0.5
# via -r requirements/edx/base.txt
edx-proctoring==3.14.0
edx-proctoring==3.13.2
# via
# -r requirements/edx/base.txt
# edx-proctoring-proctortrack