Fix staff override score
If a learner has not accessed/attempted the score override functionality didn't work. This PR is intended to fix this behaviour and the override should work regardless of learner has accessed or attempted a problem or not.
This commit is contained in:
@@ -149,6 +149,18 @@ class StudentModule(models.Model):
|
||||
def __unicode__(self):
|
||||
return unicode(repr(self))
|
||||
|
||||
@classmethod
|
||||
def get_state_by_params(cls, course_id, module_state_keys, student_id=None):
|
||||
"""
|
||||
Return all model instances that correspond to a course and module keys.
|
||||
|
||||
Student ID is optional keyword argument, if provided it narrows down the instances.
|
||||
"""
|
||||
module_states = cls.objects.filter(course_id=course_id, module_state_key__in=module_state_keys)
|
||||
if student_id:
|
||||
module_states = module_states.filter(student_id=student_id)
|
||||
return module_states
|
||||
|
||||
|
||||
class BaseStudentModuleHistory(models.Model):
|
||||
"""Abstract class containing most fields used by any class
|
||||
|
||||
@@ -6,6 +6,7 @@ import logging
|
||||
from time import time
|
||||
|
||||
from django.contrib.auth.models import User
|
||||
from django.utils.translation import ugettext_noop
|
||||
from opaque_keys.edx.keys import UsageKey
|
||||
|
||||
import dogstats_wrapper as dog_stats_api
|
||||
@@ -33,19 +34,12 @@ def perform_module_state_update(update_fcn, filter_fcn, _entry_id, course_id, ta
|
||||
"""
|
||||
Performs generic update by visiting StudentModule instances with the update_fcn provided.
|
||||
|
||||
StudentModule instances are those that match the specified `course_id` and `module_state_key`.
|
||||
If `student_identifier` is not None, it is used as an additional filter to limit the modules to those belonging
|
||||
to that student. If `student_identifier` is None, performs update on modules for all students on the specified
|
||||
problem.
|
||||
|
||||
If a `filter_fcn` is not None, it is applied to the query that has been constructed. It takes one
|
||||
argument, which is the query being filtered, and returns the filtered version of the query.
|
||||
|
||||
The `update_fcn` is called on each StudentModule that passes the resulting filtering.
|
||||
It is passed four arguments: the module_descriptor for the module pointed to by the
|
||||
module_state_key, the particular StudentModule to update, the xmodule_instance_args, and the task_input
|
||||
being passed through. If the value returned by the update function evaluates to a boolean True,
|
||||
the update is successful; False indicates the update on the particular student module failed.
|
||||
The student modules are fetched for update the `update_fcn` is called on each StudentModule
|
||||
that passes the resulting filtering. It is passed four arguments: the module_descriptor for
|
||||
the module pointed to by the module_state_key, the particular StudentModule to update, the
|
||||
xmodule_instance_args, and the task_input being passed through. If the value returned by the
|
||||
update function evaluates to a boolean True, the update is successful; False indicates the update
|
||||
on the particular student module failed.
|
||||
A raised exception indicates a fatal condition -- that no other student modules should be considered.
|
||||
|
||||
The return value is a dict containing the task's results, with the following keys:
|
||||
@@ -69,6 +63,7 @@ def perform_module_state_update(update_fcn, filter_fcn, _entry_id, course_id, ta
|
||||
problem_url = task_input.get('problem_url')
|
||||
entrance_exam_url = task_input.get('entrance_exam_url')
|
||||
student_identifier = task_input.get('student')
|
||||
override_score_task = action_name == ugettext_noop('overridden')
|
||||
problems = {}
|
||||
|
||||
# if problem_url is present make a usage key from it
|
||||
@@ -85,27 +80,11 @@ def perform_module_state_update(update_fcn, filter_fcn, _entry_id, course_id, ta
|
||||
problems = get_problems_in_section(entrance_exam_url)
|
||||
usage_keys = [UsageKey.from_string(location) for location in problems.keys()]
|
||||
|
||||
# find the modules in question
|
||||
modules_to_update = StudentModule.objects.filter(course_id=course_id, module_state_key__in=usage_keys)
|
||||
modules_to_update = _get_modules_to_update(
|
||||
course_id, usage_keys, student_identifier, filter_fcn, override_score_task
|
||||
)
|
||||
|
||||
# give the option of updating an individual student. If not specified,
|
||||
# then updates all students who have responded to a problem so far
|
||||
student = None
|
||||
if student_identifier is not None:
|
||||
# if an identifier is supplied, then look for the student,
|
||||
# and let it throw an exception if none is found.
|
||||
if "@" in student_identifier:
|
||||
student = User.objects.get(email=student_identifier)
|
||||
elif student_identifier is not None:
|
||||
student = User.objects.get(username=student_identifier)
|
||||
|
||||
if student is not None:
|
||||
modules_to_update = modules_to_update.filter(student_id=student.id)
|
||||
|
||||
if filter_fcn is not None:
|
||||
modules_to_update = filter_fcn(modules_to_update)
|
||||
|
||||
task_progress = TaskProgress(action_name, modules_to_update.count(), start_time)
|
||||
task_progress = TaskProgress(action_name, len(modules_to_update), start_time)
|
||||
task_progress.update_task_state()
|
||||
|
||||
for module_to_update in modules_to_update:
|
||||
@@ -406,3 +385,53 @@ def _get_task_id_from_xmodule_args(xmodule_instance_args):
|
||||
return UNKNOWN_TASK_ID
|
||||
else:
|
||||
return xmodule_instance_args.get('task_id', UNKNOWN_TASK_ID)
|
||||
|
||||
|
||||
def _get_modules_to_update(course_id, usage_keys, student_identifier, filter_fcn, override_score_task=False):
|
||||
"""
|
||||
Fetches a StudentModule instances for a given `course_id`, `student` object, and `usage_keys`.
|
||||
|
||||
StudentModule instances are those that match the specified `course_id` and `module_state_key`.
|
||||
If `student_identifier` is not None, it is used as an additional filter to limit the modules to those belonging
|
||||
to that student. If `student_identifier` is None, performs update on modules for all students on the specified
|
||||
problem.
|
||||
The matched instances are then applied `filter_fcn` if not None. It filters out the matched instances.
|
||||
It takes one argument, which is the query being filtered, and returns the filtered version of the query.
|
||||
If `override_score_task` is True and we there were not matching instances of StudentModule, try to create
|
||||
those instances. This is only for override scores and the use case is for learners that have missed the deadline.
|
||||
|
||||
Arguments:
|
||||
course_id(str): The unique identifier for the course.
|
||||
usage_keys(list): List of UsageKey objects
|
||||
student_identifier(str): Identifier for a student or None. The identifier can be either username or email
|
||||
filter_fcn: If it is not None, it is applied to the query that has been constructed.
|
||||
override_score_task (bool): Optional argument which indicates if it is an override score or not.
|
||||
"""
|
||||
def get_student():
|
||||
""" Fetches student instance if an identifier is provided, else return None """
|
||||
if student_identifier is None:
|
||||
return None
|
||||
|
||||
student_identifier_type = 'email' if '@' in student_identifier else 'username'
|
||||
student_query_params = {student_identifier_type: student_identifier}
|
||||
return User.objects.get(**student_query_params)
|
||||
|
||||
module_query_params = {'course_id': course_id, 'module_state_keys': usage_keys}
|
||||
|
||||
# give the option of updating an individual student. If not specified,
|
||||
# then updates all students who have responded to a problem so far
|
||||
student = get_student()
|
||||
if student:
|
||||
module_query_params['student_id'] = student.id
|
||||
|
||||
student_modules = StudentModule.get_state_by_params(**module_query_params)
|
||||
if filter_fcn is not None:
|
||||
student_modules = filter_fcn(student_modules)
|
||||
|
||||
can_create_student_modules = (override_score_task and (student_modules.count() == 0) and student is not None)
|
||||
if can_create_student_modules:
|
||||
student_modules = [
|
||||
StudentModule.objects.get_or_create(course_id=course_id, student=student, module_state_key=key)[0]
|
||||
for key in usage_keys
|
||||
]
|
||||
return student_modules
|
||||
|
||||
@@ -16,6 +16,7 @@ from mock import MagicMock, Mock, patch
|
||||
from nose.plugins.attrib import attr
|
||||
from opaque_keys.edx.locations import i4xEncoder
|
||||
|
||||
from course_modes.models import CourseMode
|
||||
from courseware.models import StudentModule
|
||||
from courseware.tests.factories import StudentModuleFactory
|
||||
from lms.djangoapps.instructor_task.exceptions import UpdateProblemModuleStateError
|
||||
@@ -31,7 +32,6 @@ from lms.djangoapps.instructor_task.tasks import (
|
||||
from lms.djangoapps.instructor_task.tasks_helper.misc import upload_ora2_data
|
||||
from lms.djangoapps.instructor_task.tests.factories import InstructorTaskFactory
|
||||
from lms.djangoapps.instructor_task.tests.test_base import InstructorTaskModuleTestCase
|
||||
from student.tests.factories import CourseEnrollmentFactory, UserFactory
|
||||
from xmodule.modulestore.exceptions import ItemNotFoundError
|
||||
|
||||
PROBLEM_URL_NAME = "test_urlname"
|
||||
@@ -69,11 +69,13 @@ class TestInstructorTasks(InstructorTaskModuleTestCase):
|
||||
task_input['score'] = score
|
||||
|
||||
course_id = course_id or self.course.id
|
||||
instructor_task = InstructorTaskFactory.create(course_id=course_id,
|
||||
requester=self.instructor,
|
||||
task_input=json.dumps(task_input, cls=i4xEncoder),
|
||||
task_key='dummy value',
|
||||
task_id=task_id)
|
||||
instructor_task = InstructorTaskFactory.create(
|
||||
course_id=course_id,
|
||||
requester=self.instructor,
|
||||
task_input=json.dumps(task_input, cls=i4xEncoder),
|
||||
task_key='dummy value',
|
||||
task_id=task_id
|
||||
)
|
||||
return instructor_task
|
||||
|
||||
def _get_xmodule_instance_args(self):
|
||||
@@ -149,19 +151,31 @@ class TestInstructorTasks(InstructorTaskModuleTestCase):
|
||||
def _create_students_with_state(self, num_students, state=None, grade=0, max_grade=1):
|
||||
"""Create students, a problem, and StudentModule objects for testing"""
|
||||
self.define_option_problem(PROBLEM_URL_NAME)
|
||||
students = [
|
||||
UserFactory.create(username='robot%d' % i, email='robot+test+%d@edx.org' % i)
|
||||
enrolled_students = self._create_and_enroll_students(num_students)
|
||||
|
||||
for student in enrolled_students:
|
||||
StudentModuleFactory.create(
|
||||
course_id=self.course.id,
|
||||
module_state_key=self.location,
|
||||
student=student,
|
||||
grade=grade,
|
||||
max_grade=max_grade,
|
||||
state=state
|
||||
)
|
||||
return enrolled_students
|
||||
|
||||
def _create_and_enroll_students(self, num_students, mode=CourseMode.DEFAULT_MODE_SLUG):
|
||||
"""Create & enroll students for testing"""
|
||||
return [
|
||||
self.create_student(username='robot%d' % i, email='robot+test+%d@edx.org' % i, mode=mode)
|
||||
for i in xrange(num_students)
|
||||
]
|
||||
for student in students:
|
||||
CourseEnrollmentFactory.create(course_id=self.course.id, user=student)
|
||||
StudentModuleFactory.create(course_id=self.course.id,
|
||||
module_state_key=self.location,
|
||||
student=student,
|
||||
grade=grade,
|
||||
max_grade=max_grade,
|
||||
state=state)
|
||||
return students
|
||||
|
||||
def _create_students_with_no_state(self, num_students):
|
||||
"""Create students and a problem for testing"""
|
||||
self.define_option_problem(PROBLEM_URL_NAME)
|
||||
enrolled_students = self._create_and_enroll_students(num_students)
|
||||
return enrolled_students
|
||||
|
||||
def _assert_num_attempts(self, students, num_attempts):
|
||||
"""Check the number attempts for all students is the same"""
|
||||
@@ -248,12 +262,15 @@ class TestOverrideScoreInstructorTask(TestInstructorTasks):
|
||||
self._test_missing_current_task(override_problem_score)
|
||||
|
||||
def test_override_undefined_course(self):
|
||||
"""Tests that override problem score raises exception with undefined course"""
|
||||
self._test_undefined_course(override_problem_score)
|
||||
|
||||
def test_override_undefined_problem(self):
|
||||
"""Tests that override problem score raises exception with undefined problem"""
|
||||
self._test_undefined_problem(override_problem_score)
|
||||
|
||||
def test_override_with_no_state(self):
|
||||
"""Tests override score with no problem state in StudentModule"""
|
||||
self._test_run_with_no_state(override_problem_score, 'overridden')
|
||||
|
||||
def test_override_with_failure(self):
|
||||
@@ -266,6 +283,9 @@ class TestOverrideScoreInstructorTask(TestInstructorTasks):
|
||||
self._test_run_with_short_error_msg(override_problem_score)
|
||||
|
||||
def test_overriding_non_scorable(self):
|
||||
"""
|
||||
Tests that override problem score raises an error if module descriptor has not `set_score` method.
|
||||
"""
|
||||
input_state = json.dumps({'done': True})
|
||||
num_students = 1
|
||||
self._create_students_with_state(num_students, input_state)
|
||||
@@ -287,7 +307,7 @@ class TestOverrideScoreInstructorTask(TestInstructorTasks):
|
||||
|
||||
def test_overriding_unaccessable(self):
|
||||
"""
|
||||
Tests rescores a problem in a course, for all students fails if user has answered a
|
||||
Tests score override for a problem in a course, for all students fails if user has answered a
|
||||
problem to which user does not have access to.
|
||||
"""
|
||||
input_state = json.dumps({'done': True})
|
||||
@@ -310,7 +330,7 @@ class TestOverrideScoreInstructorTask(TestInstructorTasks):
|
||||
|
||||
def test_overriding_success(self):
|
||||
"""
|
||||
Tests rescores a problem in a course, for all students succeeds.
|
||||
Tests score override for a problem in a course, for all students succeeds.
|
||||
"""
|
||||
mock_instance = MagicMock()
|
||||
getattr(mock_instance, 'override_problem_score').return_value = None
|
||||
@@ -334,6 +354,25 @@ class TestOverrideScoreInstructorTask(TestInstructorTasks):
|
||||
action_name='overridden'
|
||||
)
|
||||
|
||||
def test_overriding_success_with_no_state(self):
|
||||
"""
|
||||
Tests that score override is successful for a learner when they have no state.
|
||||
"""
|
||||
num_students = 1
|
||||
enrolled_students = self._create_students_with_no_state(num_students=num_students)
|
||||
task_entry = self._create_input_entry(score=1, student_ident=enrolled_students[0].username)
|
||||
|
||||
self._run_task_with_mock_celery(override_problem_score, task_entry.id, task_entry.task_id)
|
||||
self.assert_task_output(
|
||||
output=self.get_task_output(task_entry.id),
|
||||
total=num_students,
|
||||
attempted=num_students,
|
||||
succeeded=num_students,
|
||||
skipped=0,
|
||||
failed=0,
|
||||
action_name='overridden'
|
||||
)
|
||||
|
||||
|
||||
@attr(shard=3)
|
||||
@ddt.ddt
|
||||
|
||||
Reference in New Issue
Block a user