diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py index b22665b0fb..118fcfcee1 100644 --- a/common/lib/xmodule/xmodule/capa_base.py +++ b/common/lib/xmodule/xmodule/capa_base.py @@ -28,7 +28,7 @@ from xblock.scorable import ScorableXBlockMixin, Score from xmodule.capa_base_constants import RANDOMIZATION, SHOWANSWER from xmodule.exceptions import NotFoundError from xmodule.graders import ShowCorrectness -from .fields import Date, Timedelta +from .fields import Date, Timedelta, ScoreField from .progress import Progress from openedx.core.djangolib.markup import HTML, Text @@ -104,7 +104,8 @@ class CapaFields(object): attempts = Integer( help=_("Number of attempts taken by the student on this problem"), default=0, - scope=Scope.user_state) + scope=Scope.user_state + ) max_attempts = Integer( display_name=_("Maximum Attempts"), help=_("Defines the number of times a student can try to answer this problem. " @@ -183,6 +184,9 @@ class CapaFields(object): scope=Scope.user_state, default={}) input_state = Dict(help=_("Dictionary for maintaining the state of inputtypes"), scope=Scope.user_state) student_answers = Dict(help=_("Dictionary with the current student responses"), scope=Scope.user_state) + + # enforce_type is set to False here because this field is saved as a dict in the database. + score = ScoreField(help=_("Dictionary with the current student score"), scope=Scope.user_state, enforce_type=False) has_saved_answers = Boolean(help=_("Whether or not the answers have been saved since last submit"), scope=Scope.user_state) done = Boolean(help=_("Whether the student has answered the problem"), scope=Scope.user_state) @@ -292,7 +296,8 @@ class CapaMixin(ScorableXBlockMixin, CapaFields): self.set_state_from_lcp() - self.set_score(self.score_from_lcp()) + if self.score is None: + self.set_score(self.score_from_lcp()) assert self.seed is not None @@ -380,9 +385,8 @@ class CapaMixin(ScorableXBlockMixin, CapaFields): """ For now, just return weighted earned / weighted possible """ - score = self.get_score() - raw_earned = score.raw_earned - raw_possible = score.raw_possible + raw_earned = self.score.raw_earned + raw_possible = self.score.raw_possible if raw_possible > 0: if self.weight is not None: diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 905411f7b3..e437900cda 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -331,6 +331,7 @@ class CapaDescriptor(CapaFields, RawDescriptor): rescore = module_attr('rescore') reset_problem = module_attr('reset_problem') save_problem = module_attr('save_problem') + set_score = module_attr('set_score') set_state_from_lcp = module_attr('set_state_from_lcp') should_show_submit_button = module_attr('should_show_submit_button') should_show_reset_button = module_attr('should_show_reset_button') diff --git a/common/lib/xmodule/xmodule/fields.py b/common/lib/xmodule/xmodule/fields.py index 379b6e5979..d022340107 100644 --- a/common/lib/xmodule/xmodule/fields.py +++ b/common/lib/xmodule/xmodule/fields.py @@ -6,6 +6,7 @@ import time import dateutil.parser from pytz import UTC from xblock.fields import JSONField +from xblock.scorable import Score log = logging.getLogger(__name__) @@ -252,3 +253,48 @@ class RelativeTime(JSONField): return value return self.from_json(value) + + +class ScoreField(JSONField): + """ + Field for blocks that need to store a Score. XBlocks that implement + the ScorableXBlockMixin may need to store their score separately + from their problem state, specifically for use in staff override + of problem scores. + """ + MUTABLE = False + + def from_json(self, value): + if value is None: + return value + if isinstance(value, Score): + return value + + if set(value) != {'raw_earned', 'raw_possible'}: + raise TypeError('Scores must contain only a raw earned and raw possible value. Got {}'.format( + set(value) + )) + + raw_earned = value['raw_earned'] + raw_possible = value['raw_possible'] + + if raw_possible < 0: + raise ValueError( + 'Error deserializing field of type {0}: Expected a positive number for raw_possible, got {1}.'.format( + self.display_name, + raw_possible, + ) + ) + + if not (0 <= raw_earned <= raw_possible): + raise ValueError( + 'Error deserializing field of type {0}: Expected raw_earned between 0 and {1}, got {2}.'.format( + self.display_name, + raw_possible, + raw_earned + ) + ) + + return Score(raw_earned, raw_possible) + + enforce_type = from_json diff --git a/common/lib/xmodule/xmodule/js/src/capa/display.js b/common/lib/xmodule/xmodule/js/src/capa/display.js index 4a2f721004..46c2e0c933 100644 --- a/common/lib/xmodule/xmodule/js/src/capa/display.js +++ b/common/lib/xmodule/xmodule/js/src/capa/display.js @@ -241,8 +241,9 @@ totalScore ); } - } else if (attemptsUsed === 0 || totalScore === 0) { + } else if ((attemptsUsed === 0 || totalScore === 0) && curScore === 0) { // Render 'x point(s) possible' if student has not yet attempted question + // But if staff has overridden score to a non-zero number, show it if (graded) { progressTemplate = ngettext( // Translators: %(num_points)s is the number of points possible (examples: 1, 3, 10).; diff --git a/lms/djangoapps/grades/signals/handlers.py b/lms/djangoapps/grades/signals/handlers.py index 4b5e15e08a..f29fd88135 100644 --- a/lms/djangoapps/grades/signals/handlers.py +++ b/lms/djangoapps/grades/signals/handlers.py @@ -10,6 +10,7 @@ from xblock.scorable import ScorableXBlockMixin, Score from courseware.model_data import get_score, set_score from eventtracking import tracker +from lms.djangoapps.instructor_task.tasks_helper.module_state import GRADES_OVERRIDE_EVENT_TYPE from openedx.core.lib.grade_utils import is_score_higher_or_equal from student.models import user_by_anonymous_id from submissions.models import score_reset, score_set @@ -274,12 +275,9 @@ def _emit_event(kwargs): } ) - if root_type == 'edx.grades.problem.rescored': + if root_type in [GRADES_RESCORE_EVENT_TYPE, GRADES_OVERRIDE_EVENT_TYPE]: current_user = get_current_user() - if current_user is not None and hasattr(current_user, 'id'): - instructor_id = unicode(current_user.id) - else: - instructor_id = None + instructor_id = getattr(current_user, 'id', None) tracker.emit( unicode(GRADES_RESCORE_EVENT_TYPE), { @@ -289,8 +287,8 @@ def _emit_event(kwargs): 'new_weighted_earned': kwargs.get('weighted_earned'), 'new_weighted_possible': kwargs.get('weighted_possible'), 'only_if_higher': kwargs.get('only_if_higher'), - 'instructor_id': instructor_id, + 'instructor_id': unicode(instructor_id), 'event_transaction_id': unicode(get_event_transaction_id()), - 'event_transaction_type': unicode(GRADES_RESCORE_EVENT_TYPE), + 'event_transaction_type': unicode(root_type), } ) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 301b3d6927..f9911ddb4e 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -70,6 +70,7 @@ from lms.djangoapps.instructor.enrollment import ( ) from lms.djangoapps.instructor.views import INVOICE_KEY from lms.djangoapps.instructor.views.instructor_task_helpers import extract_email_features, extract_task_features +from lms.djangoapps.instructor_task.api import submit_override_score from lms.djangoapps.instructor_task.api_helper import AlreadyRunningError from lms.djangoapps.instructor_task.models import ReportStore from openedx.core.djangoapps.content.course_overviews.models import CourseOverview @@ -114,6 +115,7 @@ from util.file import ( ) from util.json_request import JsonResponse, JsonResponseBadRequest from util.views import require_global_staff +from xmodule.modulestore.django import modulestore from .tools import ( dump_module_extensions, @@ -129,6 +131,8 @@ from .tools import ( log = logging.getLogger(__name__) +TASK_SUBMISSION_OK = 'created' + def common_exceptions_400(func): """ @@ -2005,7 +2009,7 @@ def reset_student_attempts(request, course_id): response_payload['student'] = student_identifier elif all_students: lms.djangoapps.instructor_task.api.submit_reset_problem_attempts_for_all_students(request, module_state_key) - response_payload['task'] = 'created' + response_payload['task'] = TASK_SUBMISSION_OK response_payload['student'] = 'All Students' else: return HttpResponseBadRequest() @@ -2084,7 +2088,7 @@ def reset_student_attempts_for_entrance_exam(request, course_id): # pylint: dis except InvalidKeyError: return HttpResponseBadRequest(_("Course has no valid entrance exam section.")) - response_payload = {'student': student_identifier or _('All Students'), 'task': 'created'} + response_payload = {'student': student_identifier or _('All Students'), 'task': TASK_SUBMISSION_OK} return JsonResponse(response_payload) @@ -2155,7 +2159,64 @@ def rescore_problem(request, course_id): else: return HttpResponseBadRequest() - response_payload['task'] = 'created' + response_payload['task'] = TASK_SUBMISSION_OK + return JsonResponse(response_payload) + + +@transaction.non_atomic_requests +@require_POST +@ensure_csrf_cookie +@cache_control(no_cache=True, no_store=True, must_revalidate=True) +@require_level('instructor') +@require_post_params(problem_to_reset="problem urlname to reset", score='overriding score') +@common_exceptions_400 +def override_problem_score(request, course_id): + course_key = CourseKey.from_string(course_id) + score = strip_if_string(request.POST.get('score')) + problem_to_reset = strip_if_string(request.POST.get('problem_to_reset')) + student_identifier = request.POST.get('unique_student_identifier', None) + + if not problem_to_reset: + return HttpResponseBadRequest("Missing query parameter problem_to_reset.") + + if not student_identifier: + return HttpResponseBadRequest("Missing query parameter student_identifier.") + + if student_identifier is not None: + student = get_student_from_identifier(student_identifier) + else: + return _create_error_response(request, "Invalid student ID {}.".format(student_identifier)) + + try: + usage_key = UsageKey.from_string(problem_to_reset).map_into_course(course_key) + except InvalidKeyError: + return _create_error_response(request, "Unable to parse problem id {}.".format(problem_to_reset)) + + # check the user's access to this specific problem + if not has_access(request.user, "instructor", modulestore().get_item(usage_key)): + _create_error_response(request, "User {} does not have permission to override scores for problem {}.".format( + request.user.id, + problem_to_reset + )) + + response_payload = { + 'problem_to_reset': problem_to_reset, + 'student': student_identifier + } + try: + submit_override_score( + request, + usage_key, + student, + score, + ) + except NotImplementedError as exc: # if we try to override the score of a non-scorable block, catch it here + return _create_error_response(request, exc.message) + + except ValueError as exc: + return _create_error_response(request, exc.message) + + response_payload['task'] = TASK_SUBMISSION_OK return JsonResponse(response_payload) @@ -2213,7 +2274,7 @@ def rescore_entrance_exam(request, course_id): lms.djangoapps.instructor_task.api.submit_rescore_entrance_exam_for_student( request, entrance_exam_key, student, only_if_higher, ) - response_payload['task'] = 'created' + response_payload['task'] = TASK_SUBMISSION_OK return JsonResponse(response_payload) @@ -3402,3 +3463,11 @@ def _get_boolean_param(request, param_name): values to boolean values. """ return request.POST.get(param_name, False) in ['true', 'True', True] + + +def _create_error_response(request, msg): + """ + Creates the appropriate error response for the current request, + in JSON form. + """ + return JsonResponse({"error": _(msg)}, 400) diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index 0d11aece02..7078011a30 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -45,6 +45,10 @@ urlpatterns = patterns( r'^rescore_problem$', 'lms.djangoapps.instructor.views.api.rescore_problem', name="rescore_problem" + ), url( + r'^override_problem_score$', + 'lms.djangoapps.instructor.views.api.override_problem_score', + name="override_problem_score" ), url( r'^reset_student_attempts_for_entrance_exam$', 'lms.djangoapps.instructor.views.api.reset_student_attempts_for_entrance_exam', diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index a5ffbb7eeb..14368e72e2 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -567,6 +567,7 @@ def _section_student_admin(course, access): kwargs={'course_id': unicode(course_key)}, ), 'rescore_problem_url': reverse('rescore_problem', kwargs={'course_id': unicode(course_key)}), + 'override_problem_score_url': reverse('override_problem_score', kwargs={'course_id': unicode(course_key)}), 'rescore_entrance_exam_url': reverse('rescore_entrance_exam', kwargs={'course_id': unicode(course_key)}), 'student_can_skip_entrance_exam_url': reverse( 'mark_student_can_skip_entrance_exam', diff --git a/lms/djangoapps/instructor_task/api.py b/lms/djangoapps/instructor_task/api.py index 9b7101e8e7..39c5a848b8 100644 --- a/lms/djangoapps/instructor_task/api.py +++ b/lms/djangoapps/instructor_task/api.py @@ -15,6 +15,7 @@ from bulk_email.models import CourseEmail from certificates.models import CertificateGenerationHistory from lms.djangoapps.instructor_task.api_helper import ( check_arguments_for_rescoring, + check_arguments_for_overriding, check_entrance_exam_problems_for_rescoring, encode_entrance_exam_and_student_input, encode_problem_and_student_input, @@ -22,6 +23,7 @@ from lms.djangoapps.instructor_task.api_helper import ( ) from lms.djangoapps.instructor_task.models import InstructorTask from lms.djangoapps.instructor_task.tasks import ( + override_problem_score, calculate_grades_csv, calculate_may_enroll_csv, calculate_problem_grade_report, @@ -114,6 +116,28 @@ def submit_rescore_problem_for_student(request, usage_key, student, only_if_high return submit_task(request, task_type, task_class, usage_key.course_key, task_input, task_key) +def submit_override_score(request, usage_key, student, score): + """ + Request a problem score override as a background task. Only + applicable to individual users. + + The problem score will be overridden for the specified student only. + Parameters are the `course_id`, the `problem_url`, the `student` as + a User object, and the score override desired. + The url must specify the location of the problem, using i4x-type notation. + + ItemNotFoundException is raised if the problem doesn't exist, or AlreadyRunningError + if this task is already running for this student, or NotImplementedError if + the problem is not a ScorableXBlock. + """ + check_arguments_for_overriding(usage_key, score) + task_type = override_problem_score.__name__ + task_class = override_problem_score + task_input, task_key = encode_problem_and_student_input(usage_key, student) + task_input['score'] = score + return submit_task(request, task_type, task_class, usage_key.course_key, task_input, task_key) + + def submit_rescore_problem_for_all_students(request, usage_key, only_if_higher=False): # pylint: disable=invalid-name """ Request a problem to be rescored as a background task. diff --git a/lms/djangoapps/instructor_task/api_helper.py b/lms/djangoapps/instructor_task/api_helper.py index 20c3308a5e..3996af339a 100644 --- a/lms/djangoapps/instructor_task/api_helper.py +++ b/lms/djangoapps/instructor_task/api_helper.py @@ -17,6 +17,7 @@ from courseware.courses import get_problems_in_section from courseware.module_render import get_xqueue_callback_url_prefix from lms.djangoapps.instructor_task.models import PROGRESS, InstructorTask from util.db import outer_atomic +from xblock.scorable import ScorableXBlockMixin from xmodule.modulestore.django import modulestore log = logging.getLogger(__name__) @@ -263,6 +264,25 @@ def check_arguments_for_rescoring(usage_key): raise NotImplementedError(msg) +def check_arguments_for_overriding(usage_key, score): + """ + Do simple checks on the descriptor to confirm that it supports overriding + the problem score and the score passed in is not greater than the value of + the problem or less than 0. + """ + descriptor = modulestore().get_item(usage_key) + score = float(score) + + # some weirdness around initializing the descriptor requires this + if not hasattr(descriptor.__class__, 'set_score'): + msg = _("This component does not support score override.") + raise NotImplementedError(msg) + + if score < 0 or score > descriptor.max_score(): + msg = _("Scores must be between 0 and the value of the problem.") + raise ValueError(msg) + + def check_entrance_exam_problems_for_rescoring(exam_key): # pylint: disable=invalid-name """ Grabs all problem descriptors in exam and checks each descriptor to diff --git a/lms/djangoapps/instructor_task/tasks.py b/lms/djangoapps/instructor_task/tasks.py index a2ed72d57d..aa4af5fb77 100644 --- a/lms/djangoapps/instructor_task/tasks.py +++ b/lms/djangoapps/instructor_task/tasks.py @@ -45,6 +45,7 @@ from lms.djangoapps.instructor_task.tasks_helper.misc import ( from lms.djangoapps.instructor_task.tasks_helper.module_state import ( delete_problem_module_state, perform_module_state_update, + override_score_module_state, rescore_problem_module_state, reset_attempts_module_state ) @@ -80,6 +81,19 @@ def rescore_problem(entry_id, xmodule_instance_args): return run_main_task(entry_id, visit_fcn, action_name) +@task(base=BaseInstructorTask) # pylint: disable=not-callable +def override_problem_score(entry_id, xmodule_instance_args): + """ + Overrides a specific learner's score on a problem. + """ + # Translators: This is a past-tense verb that is inserted into task progress messages as {action}. + action_name = ugettext_noop('overridden') + update_fcn = partial(override_score_module_state, xmodule_instance_args) + + visit_fcn = partial(perform_module_state_update, update_fcn, None) + return run_main_task(entry_id, visit_fcn, action_name) + + @task(base=BaseInstructorTask) # pylint: disable=not-callable def reset_problem_attempts(entry_id, xmodule_instance_args): """Resets problem attempts to zero for a particular problem for all students in a course. diff --git a/lms/djangoapps/instructor_task/tasks_helper/module_state.py b/lms/djangoapps/instructor_task/tasks_helper/module_state.py index 4078305e13..e27b67f777 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/module_state.py +++ b/lms/djangoapps/instructor_task/tasks_helper/module_state.py @@ -23,6 +23,9 @@ from track.views import task_track from util.db import outer_atomic from xmodule.modulestore.django import modulestore +from xblock.runtime import KvsFieldData +from xblock.scorable import Score, ScorableXBlockMixin +from xmodule.modulestore.django import modulestore from ..exceptions import UpdateProblemModuleStateError from .runner import TaskProgress from .utils import UNKNOWN_TASK_ID, UPDATE_STATUS_FAILED, UPDATE_STATUS_SKIPPED, UPDATE_STATUS_SUCCEEDED @@ -31,6 +34,7 @@ TASK_LOG = logging.getLogger('edx.celery.task') # define value to be used in grading events GRADES_RESCORE_EVENT_TYPE = 'edx.grades.problem.rescored' +GRADES_OVERRIDE_EVENT_TYPE = 'edx.grades.problem.score_overridden' def perform_module_state_update(update_fcn, filter_fcn, _entry_id, course_id, task_input, action_name): @@ -168,8 +172,8 @@ def rescore_problem_module_state(xmodule_instance_args, module_descriptor, stude if instance is None: # Either permissions just changed, or someone is trying to be clever # and load something they shouldn't have access to. - msg = "No module {loc} for student {student}--access denied?".format( - loc=usage_key, + msg = "No module {location} for student {student}--access denied?".format( + location=usage_key, student=student ) TASK_LOG.warning(msg) @@ -220,6 +224,86 @@ def rescore_problem_module_state(xmodule_instance_args, module_descriptor, stude return UPDATE_STATUS_SUCCEEDED +@outer_atomic +def override_score_module_state(xmodule_instance_args, module_descriptor, student_module, task_input): + ''' + Takes an XModule descriptor and a corresponding StudentModule object, and + performs an override on the student's problem score. + + Throws exceptions if the override is fatal and should be aborted if in a loop. + In particular, raises UpdateProblemModuleStateError if module fails to instantiate, + or if the module doesn't support overriding, or if the score used for override + is outside the acceptable range of scores (between 0 and the max score for the + problem). + + Returns True if problem was successfully overriden for the given student, and False + if problem encountered some kind of error in overriding. + ''' + # unpack the StudentModule: + course_id = student_module.course_id + student = student_module.student + usage_key = student_module.module_state_key + + with modulestore().bulk_operations(course_id): + course = get_course_by_id(course_id) + instance = _get_module_instance_for_task( + course_id, + student, + module_descriptor, + xmodule_instance_args, + course=course + ) + + if instance is None: + # Either permissions just changed, or someone is trying to be clever + # and load something they shouldn't have access to. + msg = "No module {location} for student {student}--access denied?".format( + location=usage_key, + student=student + ) + TASK_LOG.warning(msg) + return UPDATE_STATUS_FAILED + + if not hasattr(instance, 'set_score'): + msg = "Scores cannot be overridden for this problem type." + raise UpdateProblemModuleStateError(msg) + + weighted_override_score = float(task_input['score']) + if not (0 <= weighted_override_score <= instance.max_score()): + msg = "Score must be between 0 and the maximum points available for the problem." + raise UpdateProblemModuleStateError(msg) + + # Set the tracking info before this call, because it makes downstream + # calls that create events. We retrieve and store the id here because + # the request cache will be erased during downstream calls. + create_new_event_transaction_id() + set_event_transaction_type(GRADES_OVERRIDE_EVENT_TYPE) + + problem_weight = instance.weight if instance.weight is not None else 1 + if problem_weight == 0: + msg = "Scores cannot be overridden for a problem that has a weight of zero." + raise UpdateProblemModuleStateError(msg) + else: + instance.set_score(Score( + raw_earned=weighted_override_score / problem_weight, + raw_possible=instance.max_score() / problem_weight + )) + + instance.publish_grade() + instance.save() + TASK_LOG.debug( + u"successfully processed score override for course %(course)s, problem %(loc)s " + u"and student %(student)s", + dict( + course=course_id, + loc=usage_key, + student=student + ) + ) + + return UPDATE_STATUS_SUCCEEDED + + @outer_atomic def reset_attempts_module_state(xmodule_instance_args, _module_descriptor, student_module, _task_input): """ diff --git a/lms/djangoapps/instructor_task/tests/test_api.py b/lms/djangoapps/instructor_task/tests/test_api.py index b5b4b40d68..741de82f4c 100644 --- a/lms/djangoapps/instructor_task/tests/test_api.py +++ b/lms/djangoapps/instructor_task/tests/test_api.py @@ -25,6 +25,7 @@ from lms.djangoapps.instructor_task.api import ( submit_detailed_enrollment_features_csv, submit_executive_summary_report, submit_export_ora2_data, + submit_override_score, submit_rescore_entrance_exam_for_student, submit_rescore_problem_for_all_students, submit_rescore_problem_for_student, @@ -159,6 +160,7 @@ class InstructorTaskModuleSubmitTest(InstructorTaskModuleTestCase): ), (submit_reset_problem_attempts_in_entrance_exam, 'reset_problem_attempts', {'student': True}), (submit_delete_entrance_exam_state_for_student, 'delete_problem_state', {'student': True}), + (submit_override_score, 'override_problem_score', {'student': True, 'score': 0}) ) @ddt.unpack def test_submit_task(self, task_function, expected_task_type, params=None): diff --git a/lms/djangoapps/instructor_task/tests/test_tasks.py b/lms/djangoapps/instructor_task/tests/test_tasks.py index 6ab2e6b2a8..79075427c8 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks.py @@ -25,7 +25,8 @@ from lms.djangoapps.instructor_task.tasks import ( export_ora2_data, generate_certificates, rescore_problem, - reset_problem_attempts + reset_problem_attempts, + override_problem_score ) from lms.djangoapps.instructor_task.tasks_helper.misc import upload_ora2_data from lms.djangoapps.instructor_task.tests.factories import InstructorTaskFactory @@ -54,7 +55,9 @@ class TestInstructorTasks(InstructorTaskModuleTestCase): self.instructor = self.create_instructor('instructor') self.location = self.problem_location(PROBLEM_URL_NAME) - def _create_input_entry(self, student_ident=None, use_problem_url=True, course_id=None, only_if_higher=False): + def _create_input_entry( + self, student_ident=None, use_problem_url=True, course_id=None, only_if_higher=False, score=None + ): """Creates a InstructorTask entry for testing.""" task_id = str(uuid4()) task_input = {'only_if_higher': only_if_higher} @@ -62,6 +65,8 @@ class TestInstructorTasks(InstructorTaskModuleTestCase): task_input['problem_url'] = self.location if student_ident is not None: task_input['student'] = student_ident + if score is not None: + task_input['score'] = score course_id = course_id or self.course.id instructor_task = InstructorTaskFactory.create(course_id=course_id, @@ -220,6 +225,116 @@ class TestInstructorTasks(InstructorTaskModuleTestCase): self.assertEquals(output['traceback'][-3:], "...") +class TestOverrideScoreInstructorTask(TestInstructorTasks): + """Tests instructor task to override learner's problem score""" + def assert_task_output(self, output, **expected_output): + """ + Check & compare output of the task + """ + self.assertEqual(output.get('total'), expected_output.get('total')) + self.assertEqual(output.get('attempted'), expected_output.get('attempted')) + self.assertEqual(output.get('succeeded'), expected_output.get('succeeded')) + self.assertEqual(output.get('skipped'), expected_output.get('skipped')) + self.assertEqual(output.get('failed'), expected_output.get('failed')) + self.assertEqual(output.get('action_name'), expected_output.get('action_name')) + self.assertGreater(output.get('duration_ms'), expected_output.get('duration_ms', 0)) + + def get_task_output(self, task_id): + """Get and load instructor task output""" + entry = InstructorTask.objects.get(id=task_id) + return json.loads(entry.task_output) + + def test_override_missing_current_task(self): + self._test_missing_current_task(override_problem_score) + + def test_override_undefined_course(self): + self._test_undefined_course(override_problem_score) + + def test_override_undefined_problem(self): + self._test_undefined_problem(override_problem_score) + + def test_override_with_no_state(self): + self._test_run_with_no_state(override_problem_score, 'overridden') + + def test_override_with_failure(self): + self._test_run_with_failure(override_problem_score, 'We expected this to fail') + + def test_override_with_long_error_msg(self): + self._test_run_with_long_error_msg(override_problem_score) + + def test_override_with_short_error_msg(self): + self._test_run_with_short_error_msg(override_problem_score) + + def test_overriding_non_scorable(self): + input_state = json.dumps({'done': True}) + num_students = 1 + self._create_students_with_state(num_students, input_state) + task_entry = self._create_input_entry(score=0) + mock_instance = MagicMock() + del mock_instance.set_score + with patch( + 'lms.djangoapps.instructor_task.tasks_helper.module_state.get_module_for_descriptor_internal' + ) as mock_get_module: + mock_get_module.return_value = mock_instance + with self.assertRaises(UpdateProblemModuleStateError): + self._run_task_with_mock_celery(override_problem_score, task_entry.id, task_entry.task_id) + # check values stored in table: + entry = InstructorTask.objects.get(id=task_entry.id) + output = json.loads(entry.task_output) + self.assertEquals(output['exception'], "UpdateProblemModuleStateError") + self.assertEquals(output['message'], "Scores cannot be overridden for this problem type.") + self.assertGreater(len(output['traceback']), 0) + + def test_overriding_unaccessable(self): + """ + Tests rescores 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}) + num_students = 1 + self._create_students_with_state(num_students, input_state) + task_entry = self._create_input_entry(score=0) + with patch('lms.djangoapps.instructor_task.tasks_helper.module_state.get_module_for_descriptor_internal', + return_value=None): + 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=0, + skipped=0, + failed=num_students, + action_name='overridden' + ) + + def test_overriding_success(self): + """ + Tests rescores a problem in a course, for all students succeeds. + """ + mock_instance = MagicMock() + getattr(mock_instance, 'override_problem_score').return_value = None + + num_students = 10 + self._create_students_with_state(num_students) + task_entry = self._create_input_entry(score=0) + with patch( + 'lms.djangoapps.instructor_task.tasks_helper.module_state.get_module_for_descriptor_internal' + ) as mock_get_module: + mock_get_module.return_value = mock_instance + 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 class TestRescoreInstructorTask(TestInstructorTasks): diff --git a/lms/static/js/instructor_dashboard/student_admin.js b/lms/static/js/instructor_dashboard/student_admin.js index 63e92a87c8..01ee7afe06 100644 --- a/lms/static/js/instructor_dashboard/student_admin.js +++ b/lms/static/js/instructor_dashboard/student_admin.js @@ -42,6 +42,10 @@ this.$btn_rescore_problem_if_higher_single = this.$section.find( "input[name='rescore-problem-if-higher-single']" ); + this.$btn_override_problem_score_single = this.$section.find( + "input[name='override-problem-score-single']" + ); + this.$field_select_score_single = findAndAssert(this.$section, "input[name='score-select-single']"); this.$btn_task_history_single = this.$section.find("input[name='task-history-single']"); this.$table_task_history_single = this.$section.find('.task-history-single-table'); this.$field_exam_grade = this.$section.find("input[name='entrance-exam-student-select-grade']"); @@ -408,6 +412,9 @@ this.$btn_rescore_problem_if_higher_all.click(function() { return studentadmin.rescore_problem_all(true); }); + this.$btn_override_problem_score_single.click(function() { + return studentadmin.override_problem_score_single(); + }); this.$btn_task_history_all.click(function() { var sendData; sendData = { @@ -483,6 +490,60 @@ }); }; + StudentAdmin.prototype.override_problem_score_single = function() { + var defaultErrorMessage, fullDefaultErrorMessage, fullSuccessMessage, + problemToReset, score, sendData, successMessage, uniqStudentIdentifier, + that = this; + uniqStudentIdentifier = this.$field_student_select_grade.val(); + problemToReset = this.$field_problem_select_single.val(); + score = this.$field_select_score_single.val(); + if (!uniqStudentIdentifier) { + return this.$request_err_grade.text( + gettext('Please enter a student email address or username.') + ); + } + if (!problemToReset) { + return this.$request_err_grade.text( + gettext('Please enter a problem location.') + ); + } + if (!score) { + return this.$request_err_grade.text( + gettext('Please enter a score.') + ); + } + sendData = { + unique_student_identifier: uniqStudentIdentifier, + problem_to_reset: problemToReset, + score: score + }; + successMessage = gettext("Started task to override the score for problem '<%- problem_id %>' and student '<%- student_id %>'. Click the 'Show Task Status' button to see the status of the task."); // eslint-disable-line max-len + fullSuccessMessage = _.template(successMessage)({ + student_id: uniqStudentIdentifier, + problem_id: problemToReset + }); + defaultErrorMessage = gettext("Error starting a task to override score for problem '<%- problem_id %>' for student '<%- student_id %>'. Make sure that the the score and the problem and student identifiers are complete and correct."); // eslint-disable-line max-len + fullDefaultErrorMessage = _.template(defaultErrorMessage)({ + student_id: uniqStudentIdentifier, + problem_id: problemToReset + }); + return $.ajax({ + type: 'POST', + dataType: 'json', + url: this.$btn_override_problem_score_single.data('endpoint'), + data: sendData, + success: this.clear_errors_then(function() { + return alert(fullSuccessMessage); // eslint-disable-line no-alert + }), + error: statusAjaxError(function(response) { + if (response.responseText) { + return that.$request_err_grade.text(response.responseText); + } + return that.$request_err_grade.text(fullDefaultErrorMessage); + }) + }); + }; + StudentAdmin.prototype.rescore_entrance_exam_all = function(onlyIfHigher) { var sendData, uniqStudentIdentifier, that = this; diff --git a/lms/static/js/spec/staff_debug_actions_spec.js b/lms/static/js/spec/staff_debug_actions_spec.js index cc95a867fa..f6a45fc5c7 100644 --- a/lms/static/js/spec/staff_debug_actions_spec.js +++ b/lms/static/js/spec/staff_debug_actions_spec.js @@ -11,8 +11,10 @@ define([ describe('StaffDebugActions', function() { var location = 'i4x://edX/Open_DemoX/edx_demo_course/problem/test_loc'; var locationName = 'test_loc'; - var fixtureID = 'sd_fu_' + locationName; - var $fixture = $('', {id: fixtureID, placeholder: 'userman'}); + var usernameFixtureID = 'sd_fu_' + locationName; + var $usernameFixture = $('', {id: usernameFixtureID, placeholder: 'userman'}); + var scoreFixtureID = 'sd_fs_' + locationName; + var $scoreFixture = $('', {id: scoreFixtureID, placeholder: '0'}); var escapableLocationName = 'test\.\*\+\?\^\:\$\{\}\(\)\|\]\[loc'; var escapableFixtureID = 'sd_fu_' + escapableLocationName; var $escapableFixture = $('', {id: escapableFixtureID, placeholder: 'userman'}); @@ -38,17 +40,17 @@ define([ describe('getUser', function() { it('gets the placeholder username if input field is empty', function() { - $('body').append($fixture); + $('body').append($usernameFixture); expect(StaffDebug.getUser(locationName)).toBe('userman'); - $('#' + fixtureID).remove(); + $('#' + usernameFixtureID).remove(); }); it('gets a filled in name if there is one', function() { - $('body').append($fixture); - $('#' + fixtureID).val('notuserman'); + $('body').append($usernameFixture); + $('#' + usernameFixtureID).val('notuserman'); expect(StaffDebug.getUser(locationName)).toBe('notuserman'); - $('#' + fixtureID).val(''); - $('#' + fixtureID).remove(); + $('#' + usernameFixtureID).val(''); + $('#' + usernameFixtureID).remove(); }); it('gets the placeholder name if the id has escapable characters', function() { $('body').append($escapableFixture); @@ -56,6 +58,21 @@ define([ $("input[id^='sd_fu_']").remove(); }); }); + describe('getScore', function() { + it('gets the placeholder score if input field is empty', function() { + $('body').append($scoreFixture); + expect(StaffDebug.getScore(locationName)).toBe('0'); + $('#' + scoreFixtureID).remove(); + }); + it('gets a filled in score if there is one', function() { + $('body').append($scoreFixture); + $('#' + scoreFixtureID).val('1'); + expect(StaffDebug.getScore(locationName)).toBe('1'); + + $('#' + scoreFixtureID).val(''); + $('#' + scoreFixtureID).remove(); + }); + }); describe('doInstructorDashAction success', function() { it('adds a success message to the results element after using an action', function() { $('body').append(escapableResultArea); @@ -86,7 +103,7 @@ define([ }); describe('reset', function() { it('makes an ajax call with the expected parameters', function() { - $('body').append($fixture); + $('body').append($usernameFixture); spyOn($, 'ajax'); StaffDebug.reset(locationName, location); @@ -96,17 +113,18 @@ define([ problem_to_reset: location, unique_student_identifier: 'userman', delete_module: false, - only_if_higher: undefined + only_if_higher: undefined, + score: undefined }); expect($.ajax.calls.mostRecent().args[0].url).toEqual( '/instructor/api/reset_student_attempts' ); - $('#' + fixtureID).remove(); + $('#' + usernameFixtureID).remove(); }); }); describe('deleteStudentState', function() { it('makes an ajax call with the expected parameters', function() { - $('body').append($fixture); + $('body').append($usernameFixture); spyOn($, 'ajax'); StaffDebug.deleteStudentState(locationName, location); @@ -116,18 +134,19 @@ define([ problem_to_reset: location, unique_student_identifier: 'userman', delete_module: true, - only_if_higher: undefined + only_if_higher: undefined, + score: undefined }); expect($.ajax.calls.mostRecent().args[0].url).toEqual( '/instructor/api/reset_student_attempts' ); - $('#' + fixtureID).remove(); + $('#' + usernameFixtureID).remove(); }); }); describe('rescore', function() { it('makes an ajax call with the expected parameters', function() { - $('body').append($fixture); + $('body').append($usernameFixture); spyOn($, 'ajax'); StaffDebug.rescore(locationName, location); @@ -137,17 +156,18 @@ define([ problem_to_reset: location, unique_student_identifier: 'userman', delete_module: undefined, - only_if_higher: false + only_if_higher: false, + score: undefined }); expect($.ajax.calls.mostRecent().args[0].url).toEqual( '/instructor/api/rescore_problem' ); - $('#' + fixtureID).remove(); + $('#' + usernameFixtureID).remove(); }); }); describe('rescoreIfHigher', function() { it('makes an ajax call with the expected parameters', function() { - $('body').append($fixture); + $('body').append($usernameFixture); spyOn($, 'ajax'); StaffDebug.rescoreIfHigher(locationName, location); @@ -157,12 +177,35 @@ define([ problem_to_reset: location, unique_student_identifier: 'userman', delete_module: undefined, - only_if_higher: true + only_if_higher: true, + score: undefined }); expect($.ajax.calls.mostRecent().args[0].url).toEqual( '/instructor/api/rescore_problem' ); - $('#' + fixtureID).remove(); + $('#' + usernameFixtureID).remove(); + }); + }); + describe('overrideScore', function() { + it('makes an ajax call with the expected parameters', function() { + $('body').append($usernameFixture); + $('body').append($scoreFixture); + $('#' + scoreFixtureID).val('1'); + spyOn($, 'ajax'); + StaffDebug.overrideScore(locationName, location); + + expect($.ajax.calls.mostRecent().args[0].type).toEqual('POST'); + expect($.ajax.calls.mostRecent().args[0].data).toEqual({ + problem_to_reset: location, + unique_student_identifier: 'userman', + delete_module: undefined, + only_if_higher: undefined, + score: '1' + }); + expect($.ajax.calls.mostRecent().args[0].url).toEqual( + '/instructor/api/override_problem_score' + ); + $('#' + usernameFixtureID).remove(); }); }); }); diff --git a/lms/static/js/staff_debug_actions.js b/lms/static/js/staff_debug_actions.js index 91b68b9fe2..caf4b67750 100644 --- a/lms/static/js/staff_debug_actions.js +++ b/lms/static/js/staff_debug_actions.js @@ -19,12 +19,22 @@ var StaffDebug = (function() { return uname; }; + var getScore = function(locationName) { + var sanitizedLocationName = sanitizeString(locationName); + var score = $('#sd_fs_' + sanitizedLocationName).val(); + if (score === '') { + score = $('#sd_fs_' + sanitizedLocationName).attr('placeholder'); + } + return score; + }; + var doInstructorDashAction = function(action) { var pdata = { problem_to_reset: action.location, unique_student_identifier: getUser(action.locationName), delete_module: action.delete_module, - only_if_higher: action.only_if_higher + only_if_higher: action.only_if_higher, + score: action.score }; $.ajax({ type: 'POST', @@ -105,6 +115,17 @@ var StaffDebug = (function() { }); }; + var overrideScore = function(locname, location) { + this.doInstructorDashAction({ + locationName: locname, + location: location, + method: 'override_problem_score', + success_msg: gettext('Successfully overrode problem score for {user}'), + error_msg: gettext('Could not override problem score for {user}.'), + score: getScore(locname) + }); + }; + getCurrentUrl = function() { return window.location.pathname; }; @@ -114,12 +135,14 @@ var StaffDebug = (function() { deleteStudentState: deleteStudentState, rescore: rescore, rescoreIfHigher: rescoreIfHigher, + overrideScore: overrideScore, // export for testing doInstructorDashAction: doInstructorDashAction, getCurrentUrl: getCurrentUrl, getURL: getURL, getUser: getUser, + getScore: getScore, sanitizeString: sanitizeString }; })(); @@ -142,4 +165,9 @@ $(document).ready(function() { StaffDebug.rescoreIfHigher($(this).parent().data('location-name'), $(this).parent().data('location')); return false; }); + + $courseContent.on('click', '.staff-debug-override-score', function() { + StaffDebug.overrideScore($(this).parent().data('location-name'), $(this).parent().data('location')); + return false; + }); }); diff --git a/lms/static/sass/course/courseware/_courseware.scss b/lms/static/sass/course/courseware/_courseware.scss index 6c71418b30..01b219b616 100644 --- a/lms/static/sass/course/courseware/_courseware.scss +++ b/lms/static/sass/course/courseware/_courseware.scss @@ -501,7 +501,7 @@ html.video-fullscreen { } } - .vert > .xblock-student_view { + .vert { @extend .clearfix; border-bottom: 1px solid #ddd; margin-bottom: ($baseline*0.75); diff --git a/lms/templates/instructor/instructor_dashboard_2/student_admin.html b/lms/templates/instructor/instructor_dashboard_2/student_admin.html index 66534e50eb..c443162f17 100644 --- a/lms/templates/instructor/instructor_dashboard_2/student_admin.html +++ b/lms/templates/instructor/instructor_dashboard_2/student_admin.html @@ -18,7 +18,7 @@

${_("View a specific learner's grades and progress")}


@@ -37,7 +37,7 @@

${_("Adjust a learner's grade for a specific problem")}


@@ -45,7 +45,7 @@


@@ -71,6 +71,23 @@

+ %if settings.FEATURES.get('ENABLE_INSTRUCTOR_BACKGROUND_TASKS'): +
${_("Score Override")}
+ +

+ +
+ +

+ + + + %endif + +

+
${_("Problem History")}

@@ -94,7 +111,7 @@

@@ -156,7 +173,7 @@

diff --git a/lms/templates/staff_problem_info.html b/lms/templates/staff_problem_info.html index c711af73a3..029a9a611a 100644 --- a/lms/templates/staff_problem_info.html +++ b/lms/templates/staff_problem_info.html @@ -56,7 +56,7 @@ ${block_content}