diff --git a/lms/djangoapps/grades/signals/handlers.py b/lms/djangoapps/grades/signals/handlers.py index 4a5b4d5e26..45dcf594c3 100644 --- a/lms/djangoapps/grades/signals/handlers.py +++ b/lms/djangoapps/grades/signals/handlers.py @@ -6,6 +6,7 @@ from logging import getLogger from django.dispatch import receiver from submissions.models import score_set, score_reset +from xblock.scorable import ScorableXBlockMixin, Score from courseware.model_data import get_score, set_score from eventtracking import tracker @@ -131,7 +132,14 @@ def score_published_handler(sender, block, user, raw_earned, raw_possible, only_ ) if update_score: + # Set the problem score in CSM. score_modified_time = set_score(user.id, block.location, raw_earned, raw_possible) + + # Set the problem score on the xblock. + if isinstance(block, ScorableXBlockMixin): + block.set_score(Score(raw_earned=raw_earned, raw_possible=raw_possible)) + + # Fire a signal (consumed by enqueue_subsection_update, below) PROBLEM_RAW_SCORE_CHANGED.send( sender=None, raw_earned=raw_earned, diff --git a/lms/djangoapps/instructor_task/tasks_helper.py b/lms/djangoapps/instructor_task/tasks_helper.py index 1435db9b9b..e7d49758f1 100644 --- a/lms/djangoapps/instructor_task/tasks_helper.py +++ b/lms/djangoapps/instructor_task/tasks_helper.py @@ -310,7 +310,8 @@ def perform_module_state_update(update_fcn, filter_fcn, _entry_id, course_id, ta 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. + 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. @@ -405,12 +406,18 @@ def perform_module_state_update(update_fcn, filter_fcn, _entry_id, course_id, ta def _get_task_id_from_xmodule_args(xmodule_instance_args): """Gets task_id from `xmodule_instance_args` dict, or returns default value if missing.""" - return xmodule_instance_args.get('task_id', UNKNOWN_TASK_ID) if xmodule_instance_args is not None else UNKNOWN_TASK_ID + if xmodule_instance_args is None: + return UNKNOWN_TASK_ID + else: + return xmodule_instance_args.get('task_id', UNKNOWN_TASK_ID) def _get_xqueue_callback_url_prefix(xmodule_instance_args): """Gets prefix to use when constructing xqueue_callback_url.""" - return xmodule_instance_args.get('xqueue_callback_url_prefix', '') if xmodule_instance_args is not None else '' + if xmodule_instance_args is None: + return '' + else: + return xmodule_instance_args.get('xqueue_callback_url_prefix', '') def _get_track_function_for_task(student, xmodule_instance_args=None, source_page='x_module_task'): @@ -517,83 +524,75 @@ def rescore_problem_module_state(xmodule_instance_args, module_descriptor, stude TASK_LOG.warning(msg) return UPDATE_STATUS_FAILED - if not hasattr(instance, 'rescore_problem'): - # This should also not happen, since it should be already checked in the caller, - # but check here to be sure. + # TODO: (TNL-6594) Remove this switch once rescore_problem support + # once CAPA uses ScorableXBlockMixin. + for method in ['rescore', 'rescore_problem']: + rescore_method = getattr(instance, method, None) + if rescore_method is not None: + break + else: # for-else: Neither method exists on the block. + # This should not happen, since it should be already checked in the + # caller, but check here to be sure. msg = "Specified problem does not support rescoring." 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 + # 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. event_transaction_id = create_new_event_transaction_id() set_event_transaction_type(GRADES_RESCORE_EVENT_TYPE) - result = instance.rescore_problem(only_if_higher=task_input['only_if_higher']) + result = rescore_method(only_if_higher=task_input['only_if_higher']) instance.save() - if 'success' not in result: - # don't consider these fatal, but false means that the individual call didn't complete: - TASK_LOG.warning( - u"error processing rescore call for course %(course)s, problem %(loc)s " - u"and student %(student)s: unexpected response %(msg)s", - dict( - msg=result, - course=course_id, - loc=usage_key, - student=student - ) - ) - return UPDATE_STATUS_FAILED - elif result['success'] not in ['correct', 'incorrect']: - TASK_LOG.warning( - u"error processing rescore call for course %(course)s, problem %(loc)s " - u"and student %(student)s: %(msg)s", - dict( - msg=result['success'], - course=course_id, - loc=usage_key, - student=student - ) - ) - return UPDATE_STATUS_FAILED - else: + if result is None or result.get(u'success') in {u'correct', u'incorrect'}: TASK_LOG.debug( u"successfully processed rescore call for course %(course)s, problem %(loc)s " - u"and student %(student)s: %(msg)s", + u"and student %(student)s", dict( - msg=result['success'], course=course_id, loc=usage_key, student=student ) ) - new_weighted_earned, new_weighted_possible = weighted_score( - result['new_raw_earned'], - result['new_raw_possible'], - module_descriptor.weight, - ) - # TODO: remove this context manager after completion of AN-6134 - context = contexts.course_context_from_course_id(course_id) - with tracker.get_tracker().context(GRADES_RESCORE_EVENT_TYPE, context): - tracker.emit( - unicode(GRADES_RESCORE_EVENT_TYPE), - { - 'course_id': unicode(course_id), - 'user_id': unicode(student.id), - 'problem_id': unicode(usage_key), - 'new_weighted_earned': new_weighted_earned, - 'new_weighted_possible': new_weighted_possible, - 'only_if_higher': task_input['only_if_higher'], - 'instructor_id': unicode(xmodule_instance_args['request_info']['user_id']), - 'event_transaction_id': unicode(event_transaction_id), - 'event_transaction_type': unicode(GRADES_RESCORE_EVENT_TYPE), - } + if result is not None: # Only for CAPA. This will get moved to the grade handler. + new_weighted_earned, new_weighted_possible = weighted_score( + result['new_raw_earned'] if result else None, + result['new_raw_possible'] if result else None, + module_descriptor.weight, ) - return UPDATE_STATUS_SUCCEEDED + # TODO: remove this context manager after completion of AN-6134 + context = contexts.course_context_from_course_id(course_id) + with tracker.get_tracker().context(GRADES_RESCORE_EVENT_TYPE, context): + tracker.emit( + unicode(GRADES_RESCORE_EVENT_TYPE), + { + 'course_id': unicode(course_id), + 'user_id': unicode(student.id), + 'problem_id': unicode(usage_key), + 'new_weighted_earned': new_weighted_earned, + 'new_weighted_possible': new_weighted_possible, + 'only_if_higher': task_input['only_if_higher'], + 'instructor_id': unicode(xmodule_instance_args['request_info']['user_id']), + 'event_transaction_id': unicode(event_transaction_id), + 'event_transaction_type': unicode(GRADES_RESCORE_EVENT_TYPE), + } + ) + return UPDATE_STATUS_SUCCEEDED + else: + TASK_LOG.warning( + u"error processing rescore call for course %(course)s, problem %(loc)s " + u"and student %(student)s: %(msg)s", + dict( + msg=result.get('success', result), + course=course_id, + loc=usage_key, + student=student + ) + ) + return UPDATE_STATUS_FAILED @outer_atomic diff --git a/lms/djangoapps/instructor_task/tests/test_tasks.py b/lms/djangoapps/instructor_task/tests/test_tasks.py index b4e3c4d399..ebe14cdbff 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks.py @@ -3,24 +3,24 @@ Unit tests for LMS instructor-initiated background tasks. Runs tasks on answers to course problems to validate that code paths actually work. - """ + +from functools import partial import json from uuid import uuid4 +from celery.states import SUCCESS, FAILURE +import ddt +from django.utils.translation import ugettext_noop from mock import Mock, MagicMock, patch from nose.plugins.attrib import attr -from celery.states import SUCCESS, FAILURE -from django.utils.translation import ugettext_noop -from functools import partial - -from xmodule.modulestore.exceptions import ItemNotFoundError from opaque_keys.edx.locations import i4xEncoder from courseware.models import StudentModule from courseware.tests.factories import StudentModuleFactory from student.tests.factories import UserFactory, CourseEnrollmentFactory +from xmodule.modulestore.exceptions import ItemNotFoundError from lms.djangoapps.instructor_task.models import InstructorTask from lms.djangoapps.instructor_task.tests.test_base import InstructorTaskModuleTestCase @@ -41,10 +41,16 @@ PROBLEM_URL_NAME = "test_urlname" class TestTaskFailure(Exception): + """ + An example exception to indicate failure of a mocked task. + """ pass class TestInstructorTasks(InstructorTaskModuleTestCase): + """ + Ensure tasks behave as expected. + """ def setUp(self): super(TestInstructorTasks, self).setUp() @@ -219,6 +225,7 @@ class TestInstructorTasks(InstructorTaskModuleTestCase): @attr(shard=3) +@ddt.ddt class TestRescoreInstructorTask(TestInstructorTasks): """Tests problem-rescoring instructor task.""" @@ -267,6 +274,7 @@ class TestRescoreInstructorTask(TestInstructorTasks): task_entry = self._create_input_entry() mock_instance = MagicMock() del mock_instance.rescore_problem + del mock_instance.rescore with patch('lms.djangoapps.instructor_task.tasks_helper.get_module_for_descriptor_internal') as mock_get_module: mock_get_module.return_value = mock_instance with self.assertRaises(UpdateProblemModuleStateError): @@ -300,22 +308,24 @@ class TestRescoreInstructorTask(TestInstructorTasks): action_name='rescored' ) - def test_rescoring_success(self): + @ddt.data( + ('rescore', None), + ('rescore_problem', {'success': 'correct', 'new_raw_earned': 1, 'new_raw_possible': 1}) + ) + @ddt.unpack + def test_rescoring_success(self, rescore_method, rescore_result): """ Tests rescores a problem in a course, for all students succeeds. """ + mock_instance = MagicMock() + other_method = ({'rescore', 'rescore_problem'} - {rescore_method}).pop() + getattr(mock_instance, rescore_method).return_value = rescore_result + delattr(mock_instance, other_method) + input_state = json.dumps({'done': True}) num_students = 10 self._create_students_with_state(num_students, input_state) task_entry = self._create_input_entry() - mock_instance = Mock() - mock_instance.rescore_problem = Mock( - return_value={ - 'success': 'correct', - 'new_raw_earned': 1, - 'new_raw_possible': 1, - } - ) with patch('lms.djangoapps.instructor_task.tasks_helper.get_module_for_descriptor_internal') as mock_get_module: mock_get_module.return_value = mock_instance self._run_task_with_mock_celery(rescore_problem, task_entry.id, task_entry.task_id) @@ -340,6 +350,7 @@ class TestRescoreInstructorTask(TestInstructorTasks): task_entry = self._create_input_entry() mock_instance = Mock() mock_instance.rescore_problem = Mock(return_value={'success': 'bogus'}) + del mock_instance.rescore with patch('lms.djangoapps.instructor_task.tasks_helper.get_module_for_descriptor_internal') as mock_get_module: mock_get_module.return_value = mock_instance self._run_task_with_mock_celery(rescore_problem, task_entry.id, task_entry.task_id) @@ -364,6 +375,7 @@ class TestRescoreInstructorTask(TestInstructorTasks): task_entry = self._create_input_entry() mock_instance = Mock() mock_instance.rescore_problem = Mock(return_value={'bogus': 'value'}) + del mock_instance.rescore with patch('lms.djangoapps.instructor_task.tasks_helper.get_module_for_descriptor_internal') as mock_get_module: mock_get_module.return_value = mock_instance self._run_task_with_mock_celery(rescore_problem, task_entry.id, task_entry.task_id) diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index 5e0d875842..4e7e8d5d4a 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -13,44 +13,43 @@ import shutil from datetime import datetime import urllib +from django.conf import settings +from django.core.urlresolvers import reverse +from django.test.utils import override_settings import ddt from freezegun import freeze_time from mock import Mock, patch, MagicMock from nose.plugins.attrib import attr +from pytz import UTC import tempfile import unicodecsv -from django.core.urlresolvers import reverse -from django.test.utils import override_settings from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory from certificates.models import CertificateStatuses, GeneratedCertificate from certificates.tests.factories import GeneratedCertificateFactory, CertificateWhitelistFactory from course_modes.models import CourseMode from courseware.tests.factories import InstructorFactory -from lms.djangoapps.instructor_task.tests.test_base import ( - InstructorTaskCourseTestCase, - TestReportMixin, - InstructorTaskModuleTestCase -) +from instructor_analytics.basic import UNAVAILABLE +from lms.djangoapps.teams.tests.factories import CourseTeamFactory, CourseTeamMembershipFactory +from lms.djangoapps.verify_student.tests.factories import SoftwareSecurePhotoVerificationFactory from openedx.core.djangoapps.course_groups.models import CourseUserGroupPartitionGroup, CohortMembership -from django.conf import settings -from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase -from pytz import UTC - -from student.tests.factories import CourseEnrollmentFactory, UserFactory from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory import openedx.core.djangoapps.user_api.course_tag.api as course_tag_api from openedx.core.djangoapps.user_api.partition_schemes import RandomUserPartitionScheme -from shoppingcart.models import Order, PaidCourseRegistration, CourseRegistrationCode, Invoice, \ +from openedx.core.djangoapps.util.testing import ContentGroupTestCase, TestConditionalContent +from shoppingcart.models import ( + Order, PaidCourseRegistration, CourseRegistrationCode, Invoice, CourseRegistrationCodeInvoiceItem, InvoiceTransaction, Coupon -from student.tests.factories import UserFactory, CourseModeFactory +) from student.models import CourseEnrollment, CourseEnrollmentAllowed, ManualEnrollmentAudit, ALLOWEDTOENROLL_TO_ENROLLED -from lms.djangoapps.verify_student.tests.factories import SoftwareSecurePhotoVerificationFactory +from student.tests.factories import CourseEnrollmentFactory, CourseModeFactory, UserFactory +from survey.models import SurveyForm, SurveyAnswer +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.partitions.partitions import Group, UserPartition -from lms.djangoapps.instructor_task.models import ReportStore -from survey.models import SurveyForm, SurveyAnswer -from lms.djangoapps.instructor_task.tasks_helper import ( + +from ..models import ReportStore +from ..tasks_helper import ( cohort_students_and_upload, upload_problem_responses_csv, upload_grades_csv, @@ -65,9 +64,12 @@ from lms.djangoapps.instructor_task.tasks_helper import ( UPDATE_STATUS_FAILED, UPDATE_STATUS_SUCCEEDED, ) -from instructor_analytics.basic import UNAVAILABLE -from openedx.core.djangoapps.util.testing import ContentGroupTestCase, TestConditionalContent -from teams.tests.factories import CourseTeamFactory, CourseTeamMembershipFactory + +from lms.djangoapps.instructor_task.tests.test_base import ( + InstructorTaskCourseTestCase, + TestReportMixin, + InstructorTaskModuleTestCase +) class InstructorGradeReportTestCase(TestReportMixin, InstructorTaskCourseTestCase): diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 41e6b3fa8f..8463272405 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -209,7 +209,7 @@ py2neo==3.1.2 # Support for plugins web-fragments==0.2.1 -xblock==0.4.14 +xblock==0.5.0 # Third Party XBlocks edx-sga==0.6.2