diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 97de18b825..47223681f2 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -413,7 +413,7 @@ def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disa if status in {'generating', 'ready', 'notpassing', 'restricted', 'auditing', 'unverified'}: cert_grade_percent = -1 persisted_grade_percent = -1 - persisted_grade = CourseGradeFactory().read(user, course=course_overview) + persisted_grade = CourseGradeFactory().read(user, course=course_overview, create_if_needed=False) if persisted_grade is not None: persisted_grade_percent = persisted_grade.percent diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index 49aa217082..aa400bc7da 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -237,18 +237,18 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase): # # of sql queries to default, # # of mongo queries, # ) - ('no_overrides', 1, True, False): (26, 1), - ('no_overrides', 2, True, False): (26, 1), - ('no_overrides', 3, True, False): (26, 1), - ('ccx', 1, True, False): (26, 1), - ('ccx', 2, True, False): (26, 1), - ('ccx', 3, True, False): (26, 1), - ('no_overrides', 1, False, False): (26, 1), - ('no_overrides', 2, False, False): (26, 1), - ('no_overrides', 3, False, False): (26, 1), - ('ccx', 1, False, False): (26, 1), - ('ccx', 2, False, False): (26, 1), - ('ccx', 3, False, False): (26, 1), + ('no_overrides', 1, True, False): (19, 1), + ('no_overrides', 2, True, False): (19, 1), + ('no_overrides', 3, True, False): (19, 1), + ('ccx', 1, True, False): (19, 1), + ('ccx', 2, True, False): (19, 1), + ('ccx', 3, True, False): (19, 1), + ('no_overrides', 1, False, False): (19, 1), + ('no_overrides', 2, False, False): (19, 1), + ('no_overrides', 3, False, False): (19, 1), + ('ccx', 1, False, False): (19, 1), + ('ccx', 2, False, False): (19, 1), + ('ccx', 3, False, False): (19, 1), } @@ -260,19 +260,19 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase): __test__ = True TEST_DATA = { - ('no_overrides', 1, True, False): (26, 3), - ('no_overrides', 2, True, False): (26, 3), - ('no_overrides', 3, True, False): (26, 3), - ('ccx', 1, True, False): (26, 3), - ('ccx', 2, True, False): (26, 3), - ('ccx', 3, True, False): (26, 3), - ('ccx', 1, True, True): (27, 3), - ('ccx', 2, True, True): (27, 3), - ('ccx', 3, True, True): (27, 3), - ('no_overrides', 1, False, False): (26, 3), - ('no_overrides', 2, False, False): (26, 3), - ('no_overrides', 3, False, False): (26, 3), - ('ccx', 1, False, False): (26, 3), - ('ccx', 2, False, False): (26, 3), - ('ccx', 3, False, False): (26, 3), + ('no_overrides', 1, True, False): (19, 3), + ('no_overrides', 2, True, False): (19, 3), + ('no_overrides', 3, True, False): (19, 3), + ('ccx', 1, True, False): (19, 3), + ('ccx', 2, True, False): (19, 3), + ('ccx', 3, True, False): (19, 3), + ('ccx', 1, True, True): (20, 3), + ('ccx', 2, True, True): (20, 3), + ('ccx', 3, True, True): (20, 3), + ('no_overrides', 1, False, False): (19, 3), + ('no_overrides', 2, False, False): (19, 3), + ('no_overrides', 3, False, False): (19, 3), + ('ccx', 1, False, False): (19, 3), + ('ccx', 2, False, False): (19, 3), + ('ccx', 3, False, False): (19, 3), } diff --git a/lms/djangoapps/ccx/tests/test_views.py b/lms/djangoapps/ccx/tests/test_views.py index b895a4294c..b388e040cb 100644 --- a/lms/djangoapps/ccx/tests/test_views.py +++ b/lms/djangoapps/ccx/tests/test_views.py @@ -36,6 +36,7 @@ from lms.djangoapps.ccx.tests.factories import CcxFactory from lms.djangoapps.ccx.tests.utils import CcxTestCase, flatten from lms.djangoapps.ccx.utils import ccx_course, is_email from lms.djangoapps.ccx.views import get_date +from lms.djangoapps.grades.tasks import compute_all_grades_for_course from lms.djangoapps.instructor.access import allow_access, list_with_level from request_cache.middleware import RequestCache from student.models import CourseEnrollment, CourseEnrollmentAllowed @@ -110,6 +111,8 @@ def setup_students_and_grades(context): module_state_key=problem.location ) + compute_all_grades_for_course.apply_async(kwargs={'course_key': unicode(context.course.id)}) + def unhide(unit): """ diff --git a/lms/djangoapps/ccx/utils.py b/lms/djangoapps/ccx/utils.py index 279a6d54e9..9a34150e5c 100644 --- a/lms/djangoapps/ccx/utils.py +++ b/lms/djangoapps/ccx/utils.py @@ -265,12 +265,6 @@ def ccx_students_enrolling_center(action, identifiers, email_students, course_ke return errors -def prep_course_for_grading(course, request): - """Set up course module for overrides to function properly""" - course._field_data_cache = {} # pylint: disable=protected-access - course.set_grading_policy(course.grading_policy) - - @contextmanager def ccx_course(ccx_locator): """Create a context in which the course identified by course_locator exists diff --git a/lms/djangoapps/ccx/views.py b/lms/djangoapps/ccx/views.py index 59a956f97c..1a43a560fe 100644 --- a/lms/djangoapps/ccx/views.py +++ b/lms/djangoapps/ccx/views.py @@ -46,7 +46,6 @@ from lms.djangoapps.ccx.utils import ( get_ccx_for_coach, get_date, parse_date, - prep_course_for_grading ) from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory from lms.djangoapps.instructor.enrollment import enroll_email, get_email_params @@ -519,7 +518,6 @@ def ccx_gradebook(request, course, ccx=None): ccx_key = CCXLocator.from_course_locator(course.id, unicode(ccx.id)) with ccx_course(ccx_key) as course: - prep_course_for_grading(course, request) student_info, page = get_grade_book_page(request, course, course_key=ccx_key) return render_to_response('courseware/gradebook.html', { @@ -547,7 +545,6 @@ def ccx_grades_csv(request, course, ccx=None): ccx_key = CCXLocator.from_course_locator(course.id, unicode(ccx.id)) with ccx_course(ccx_key) as course: - prep_course_for_grading(course, request) enrolled_students = User.objects.filter( courseenrollment__course_id=ccx_key, diff --git a/lms/djangoapps/certificates/management/commands/fix_ungraded_certs.py b/lms/djangoapps/certificates/management/commands/fix_ungraded_certs.py index 34a017344f..117bdd7334 100644 --- a/lms/djangoapps/certificates/management/commands/fix_ungraded_certs.py +++ b/lms/djangoapps/certificates/management/commands/fix_ungraded_certs.py @@ -51,7 +51,7 @@ class Command(BaseCommand): course = courses.get_course_by_id(course_id) for cert in ungraded: # grade the student - grade = CourseGradeFactory().create(cert.user, course) + grade = CourseGradeFactory().read(cert.user, course) log.info('grading %s - %s', cert.user, grade.percent) cert.grade = grade.percent if not options['noop']: diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py index 3d82bc2859..5dbc394713 100644 --- a/lms/djangoapps/certificates/queue.py +++ b/lms/djangoapps/certificates/queue.py @@ -269,7 +269,7 @@ class XQueueCertInterface(object): self.request.session = {} is_whitelisted = self.whitelist.filter(user=student, course_id=course_id, whitelist=True).exists() - course_grade = CourseGradeFactory().create(student, course) + course_grade = CourseGradeFactory().read(student, course) enrollment_mode, __ = CourseEnrollment.enrollment_mode_for_user(student, course_id) mode_is_verified = enrollment_mode in GeneratedCertificate.VERIFIED_CERTS_MODES user_is_verified = SoftwareSecurePhotoVerification.user_is_verified(student) diff --git a/lms/djangoapps/certificates/tests/test_queue.py b/lms/djangoapps/certificates/tests/test_queue.py index dac452ceca..e6fed1a2f8 100644 --- a/lms/djangoapps/certificates/tests/test_queue.py +++ b/lms/djangoapps/certificates/tests/test_queue.py @@ -265,7 +265,7 @@ class XQueueCertInterfaceAddCertificateTest(ModuleStoreTestCase): ) # Run grading/cert generation again - with mock_passing_grade(grade_pass=grade): + with mock_passing_grade(letter_grade=grade): with patch.object(XQueueInterface, 'send_to_queue') as mock_send: mock_send.return_value = (0, None) self.xqueue.add_cert(self.user_2, self.course.id) diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py index 0fe41cd0e0..e86788d28c 100644 --- a/lms/djangoapps/courseware/tests/test_submitting_problems.py +++ b/lms/djangoapps/courseware/tests/test_submitting_problems.py @@ -29,6 +29,7 @@ from course_modes.models import CourseMode from courseware.models import BaseStudentModuleHistory, StudentModule from courseware.tests.helpers import LoginEnrollmentTestCase from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory +from lms.djangoapps.grades.tasks import compute_all_grades_for_course from openedx.core.djangoapps.credit.api import get_credit_requirement_status, set_credit_requirements from openedx.core.djangoapps.credit.models import CreditCourse, CreditProvider from openedx.core.djangoapps.user_api.tests.factories import UserCourseTagFactory @@ -143,6 +144,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl COURSE_NAME = "test_course" ENABLED_CACHES = ['default', 'mongo_metadata_inheritance', 'loc_cache'] + ENABLED_SIGNALS = ['course_published'] def setUp(self): super(TestSubmittingProblems, self).setUp() @@ -156,25 +158,6 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl self.enroll(self.course) self.student_user = User.objects.get(email=self.student) self.factory = RequestFactory() - # Disable the score change signal to prevent other components from being pulled into tests. - self.score_changed_signal_patch = patch( - 'lms.djangoapps.grades.signals.handlers.PROBLEM_WEIGHTED_SCORE_CHANGED.send' - ) - self.score_changed_signal_patch.start() - - def tearDown(self): - super(TestSubmittingProblems, self).tearDown() - self._stop_signal_patch() - - def _stop_signal_patch(self): - """ - Stops the signal patch for the PROBLEM_WEIGHTED_SCORE_CHANGED event. - In case a test wants to test with the event actually - firing. - """ - if self.score_changed_signal_patch: - self.score_changed_signal_patch.stop() - self.score_changed_signal_patch = None def add_dropdown_to_section(self, section_location, name, num_inputs=2): """ @@ -278,7 +261,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl """ Return CourseGrade for current user and course. """ - return CourseGradeFactory().create(self.student_user, self.course) + return CourseGradeFactory().read(self.student_user, self.course) def check_grade_percent(self, percent): """ @@ -424,6 +407,7 @@ class TestCourseGrader(TestSubmittingProblems): ] } self.add_grading_policy(grading_policy) + compute_all_grades_for_course.apply_async(kwargs={'course_key': unicode(self.course.id)}) def dropping_setup(self): """ @@ -597,10 +581,6 @@ class TestCourseGrader(TestSubmittingProblems): self.check_grade_percent(0.67) self.assertEqual(self.get_course_grade().letter_grade, 'B') - # But now, set the score with the submissions API and watch - # as it overrides the score read from StudentModule and our - # student gets an A instead. - self._stop_signal_patch() student_item = { 'student_id': anonymous_id_for_user(self.student_user, self.course.id), 'course_id': unicode(self.course.id), @@ -619,7 +599,6 @@ class TestCourseGrader(TestSubmittingProblems): self.basic_setup() self.submit_question_answer('p1', {'2_1': 'Correct'}) self.submit_question_answer('p2', {'2_1': 'Correct'}) - self.submit_question_answer('p3', {'2_1': 'Incorrect'}) with patch('submissions.api.get_scores') as mock_get_scores: mock_get_scores.return_value = { @@ -629,7 +608,7 @@ class TestCourseGrader(TestSubmittingProblems): 'created_at': now(), }, } - self.get_course_grade() + self.submit_question_answer('p3', {'2_1': 'Incorrect'}) # Verify that the submissions API was sent an anonymized student ID mock_get_scores.assert_called_with( @@ -764,7 +743,6 @@ class TestCourseGrader(TestSubmittingProblems): req_status = get_credit_requirement_status(self.course.id, self.student_user.username, 'grade', 'grade') self.assertEqual(req_status[0]["status"], None) - self._stop_signal_patch() self.submit_question_answer('p1', {'2_1': 'Correct'}) self.submit_question_answer('p2', {'2_1': 'Correct'}) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 931d20c7ad..1b772ae3ef 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -42,8 +42,6 @@ from django.test.utils import override_settings from lms.djangoapps.commerce.utils import EcommerceService # pylint: disable=import-error from lms.djangoapps.grades.config.waffle import waffle as grades_waffle from lms.djangoapps.grades.config.waffle import ASSUME_ZERO_GRADE_IF_ABSENT -from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory -from lms.djangoapps.grades.tests.utils import mock_get_score from milestones.tests.utils import MilestonesTestCaseMixin from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locations import Location @@ -1399,7 +1397,7 @@ class ProgressPageTests(ProgressPageBaseTests): self.course.save() self.store.update_item(self.course, self.user.id) - with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.create') as mock_create: + with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_create: course_grade = mock_create.return_value course_grade.passed = True course_grade.summary = {'grade': 'Pass', 'percent': 0.75, 'section_breakdown': [], 'grade_breakdown': {}} @@ -1442,7 +1440,7 @@ class ProgressPageTests(ProgressPageBaseTests): # Enable certificate generation for this course certs_api.set_cert_generation_enabled(self.course.id, True) - with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.create') as mock_create: + with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_create: course_grade = mock_create.return_value course_grade.passed = True course_grade.summary = {'grade': 'Pass', 'percent': 0.75, 'section_breakdown': [], 'grade_breakdown': {}} @@ -1458,9 +1456,10 @@ class ProgressPageTests(ProgressPageBaseTests): """Test that query counts remain the same for self-paced and instructor-paced courses.""" SelfPacedConfiguration(enabled=self_paced_enabled).save() self.setup_course(self_paced=self_paced) - with self.assertNumQueries(43, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST), check_mongo_calls(1): + with self.assertNumQueries(36, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST), check_mongo_calls(1): self._get_progress_page() + @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) @ddt.data( (False, 43, 27), (True, 36, 23) @@ -1504,7 +1503,7 @@ class ProgressPageTests(ProgressPageBaseTests): 'lms.djangoapps.verify_student.models.SoftwareSecurePhotoVerification.user_is_verified' ) as user_verify: user_verify.return_value = user_verified - with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.create') as mock_create: + with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_create: course_grade = mock_create.return_value course_grade.passed = True course_grade.summary = { @@ -1548,7 +1547,7 @@ class ProgressPageTests(ProgressPageBaseTests): self.course.save() self.store.update_item(self.course, self.user.id) - with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.create') as mock_create: + with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_create: course_grade = mock_create.return_value course_grade.passed = True course_grade.summary = { @@ -1568,7 +1567,7 @@ class ProgressPageTests(ProgressPageBaseTests): "http://www.example.com/certificate.pdf", "honor" ) - with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.create') as mock_create: + with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_create: course_grade = mock_create.return_value course_grade.passed = True course_grade.summary = {'grade': 'Pass', 'percent': 0.75, 'section_breakdown': [], 'grade_breakdown': {}} @@ -1586,7 +1585,7 @@ class ProgressPageTests(ProgressPageBaseTests): self.assertTrue(self.client.login(username=user.username, password='test')) CourseEnrollmentFactory(user=user, course_id=self.course.id, mode=CourseMode.AUDIT) - with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.create') as mock_create: + with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_create: course_grade = mock_create.return_value course_grade.passed = True course_grade.summary = {'grade': 'Pass', 'percent': 0.75, 'section_breakdown': [], 'grade_breakdown': {}} @@ -2090,7 +2089,7 @@ class GenerateUserCertTests(ModuleStoreTestCase): status=CertificateStatuses.generating, mode='verified' ) - with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.create') as mock_create: + with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_create: course_grade = mock_create.return_value course_grade.passed = True course_grade.summary = {'grade': 'Pass', 'percent': 0.75} @@ -2111,7 +2110,7 @@ class GenerateUserCertTests(ModuleStoreTestCase): mode='verified' ) - with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.create') as mock_create: + with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_create: course_grade = mock_create.return_value course_grade.passed = True course_grade.summay = {'grade': 'Pass', 'percent': 0.75} diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py index 22a91e59be..cdc45db8e0 100644 --- a/lms/djangoapps/courseware/views/index.py +++ b/lms/djangoapps/courseware/views/index.py @@ -425,7 +425,7 @@ class CoursewareIndex(View): if course_has_entrance_exam(self.course) and getattr(self.chapter, 'is_entrance_exam', False): courseware_context['entrance_exam_passed'] = user_has_passed_entrance_exam(self.effective_user, self.course) courseware_context['entrance_exam_current_score'] = get_entrance_exam_score_ratio( - CourseGradeFactory().create(self.effective_user, self.course), + CourseGradeFactory().read(self.effective_user, self.course), get_entrance_exam_usage_key(self.course), ) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 16bdb7b21c..a07ebac5ce 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -60,7 +60,6 @@ from enrollment.api import add_enrollment from eventtracking import tracker from ipware.ip import get_ip from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException -from lms.djangoapps.ccx.utils import prep_course_for_grading from lms.djangoapps.courseware.exceptions import CourseAccessRedirect, Redirect from lms.djangoapps.experiments.utils import get_experiment_user_metadata_context from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory @@ -922,12 +921,11 @@ def _progress(request, course_key, student_id): if request.user.id != student.id: # refetch the course as the assumed student course = get_course_with_access(student, 'load', course_key, check_if_enrolled=True) - prep_course_for_grading(course, request) # NOTE: To make sure impersonation by instructor works, use # student instead of request.user in the rest of the function. - course_grade = CourseGradeFactory().create(student, course) + course_grade = CourseGradeFactory().read(student, course) courseware_summary = course_grade.chapter_grades.values() studio_url = get_studio_url(course, 'settings/grading') @@ -1015,7 +1013,7 @@ def _get_cert_data(student, course, enrollment_mode, course_grade=None): certificates_enabled_for_course = certs_api.cert_generation_enabled(course.id) if course_grade is None: - course_grade = CourseGradeFactory().create(student, course) + course_grade = CourseGradeFactory().read(student, course) if not auto_certs_api.can_show_certificate_message(course, student, course_grade, certificates_enabled_for_course): return @@ -1290,7 +1288,7 @@ def is_course_passed(student, course, course_grade=None): returns bool value """ if course_grade is None: - course_grade = CourseGradeFactory().create(student, course) + course_grade = CourseGradeFactory().read(student, course) return course_grade.passed diff --git a/lms/djangoapps/gating/tests/test_integration.py b/lms/djangoapps/gating/tests/test_integration.py index a572cb25dd..e833606526 100644 --- a/lms/djangoapps/gating/tests/test_integration.py +++ b/lms/djangoapps/gating/tests/test_integration.py @@ -149,7 +149,7 @@ class TestGatedContent(MilestonesTestCaseMixin, SharedModuleStoreTestCase): all problems in the course, whether or not they are currently gated. """ - course_grade = CourseGradeFactory().create(user, self.course) + course_grade = CourseGradeFactory().read(user, self.course) for prob in [self.gating_prob1, self.gated_prob2, self.prob3]: self.assertIn(prob.location, course_grade.problem_scores) diff --git a/lms/djangoapps/grades/api/tests/test_views.py b/lms/djangoapps/grades/api/tests/test_views.py index 2aa6f8502b..2a57299567 100644 --- a/lms/djangoapps/grades/api/tests/test_views.py +++ b/lms/djangoapps/grades/api/tests/test_views.py @@ -15,7 +15,7 @@ from rest_framework.test import APITestCase from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory from lms.djangoapps.courseware.tests.factories import GlobalStaffFactory, StaffFactory -from lms.djangoapps.grades.tests.utils import mock_get_score +from lms.djangoapps.grades.tests.utils import mock_passing_grade from student.tests.factories import CourseEnrollmentFactory, UserFactory from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, SharedModuleStoreTestCase @@ -149,7 +149,7 @@ class CurrentGradeViewTest(SharedModuleStoreTestCase, APITestCase): """ Test that a user can successfully request her own grade. """ - with check_mongo_calls(6): + with check_mongo_calls(3): resp = self.client.get(self.get_url(self.student.username)) self.assertEqual(resp.status_code, status.HTTP_200_OK) @@ -286,22 +286,21 @@ class CurrentGradeViewTest(SharedModuleStoreTestCase, APITestCase): self.assertEqual(resp.data, expected_data) # pylint: disable=no-member @ddt.data( - ((2, 5), {'letter_grade': None, 'percent': 0.4, 'passed': False}), - ((5, 5), {'letter_grade': 'Pass', 'percent': 1, 'passed': True}), + ({'letter_grade': None, 'percent': 0.4, 'passed': False}), + ({'letter_grade': 'Pass', 'percent': 1, 'passed': True}), ) - @ddt.unpack - def test_grade(self, grade, result): + def test_grade(self, grade): """ Test that the user gets her grade in case she answered tests with an insufficient score. """ - with mock_get_score(*grade): + with mock_passing_grade(letter_grade=grade['letter_grade'], percent=grade['percent']): resp = self.client.get(self.get_url(self.student.username)) self.assertEqual(resp.status_code, status.HTTP_200_OK) expected_data = { 'username': self.student.username, 'course_key': str(self.course_key), } - expected_data.update(result) + expected_data.update(grade) self.assertEqual(resp.data, [expected_data]) # pylint: disable=no-member diff --git a/lms/djangoapps/grades/api/views.py b/lms/djangoapps/grades/api/views.py index 4fd1f5d7c5..81f05ad7dc 100644 --- a/lms/djangoapps/grades/api/views.py +++ b/lms/djangoapps/grades/api/views.py @@ -11,7 +11,6 @@ from rest_framework.generics import GenericAPIView, ListAPIView from rest_framework.response import Response from courseware.access import has_access -from lms.djangoapps.ccx.utils import prep_course_for_grading from lms.djangoapps.courseware import courses from lms.djangoapps.courseware.exceptions import CourseAccessRedirect from lms.djangoapps.grades.api.serializers import GradingPolicySerializer @@ -184,8 +183,7 @@ class UserGradeView(GradeViewMixin, GenericAPIView): # or a 404 if the requested user does not exist. return grade_user - prep_course_for_grading(course, request) - course_grade = CourseGradeFactory().create(grade_user, course) + course_grade = CourseGradeFactory().read(grade_user, course) return Response([{ 'username': grade_user.username, 'course_key': course_id, diff --git a/lms/djangoapps/grades/config/__init__.py b/lms/djangoapps/grades/config/__init__.py index 5efb977cfd..1ecd4acb84 100644 --- a/lms/djangoapps/grades/config/__init__.py +++ b/lms/djangoapps/grades/config/__init__.py @@ -1,3 +1,5 @@ +from django.conf import settings + from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag from lms.djangoapps.grades.config.waffle import waffle as waffle_func, ASSUME_ZERO_GRADE_IF_ABSENT @@ -6,7 +8,12 @@ def assume_zero_if_absent(course_key): """ Returns whether an absent grade should be assumed to be zero. """ - return should_persist_grades(course_key) and waffle_func().is_enabled(ASSUME_ZERO_GRADE_IF_ABSENT) + return ( + should_persist_grades(course_key) and ( + settings.FEATURES.get('ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS') or + waffle_func().is_enabled(ASSUME_ZERO_GRADE_IF_ABSENT) + ) + ) def should_persist_grades(course_key): diff --git a/lms/djangoapps/grades/course_data.py b/lms/djangoapps/grades/course_data.py index e7993b627f..a7d954a5ab 100644 --- a/lms/djangoapps/grades/course_data.py +++ b/lms/djangoapps/grades/course_data.py @@ -92,8 +92,9 @@ class CourseData(object): def edited_on(self): # get course block from structure only; subtree_edited_on field on modulestore's course block isn't optimized. structure = self._effective_structure - course_block = structure[self.location] - return getattr(course_block, 'subtree_edited_on', None) + if structure: + course_block = structure[self.location] + return getattr(course_block, 'subtree_edited_on', None) def __unicode__(self): return u'Course: course_key: {}'.format(self.course_key) diff --git a/lms/djangoapps/grades/course_grade.py b/lms/djangoapps/grades/course_grade.py index b438001d07..5e10ce0336 100644 --- a/lms/djangoapps/grades/course_grade.py +++ b/lms/djangoapps/grades/course_grade.py @@ -7,6 +7,7 @@ from collections import OrderedDict, defaultdict from django.conf import settings from lazy import lazy +from ccx_keys.locator import CCXLocator from xmodule import block_metadata_utils from .subsection_grade import ZeroSubsectionGrade @@ -21,7 +22,7 @@ class CourseGradeBase(object): """ Base class for Course Grades. """ - def __init__(self, user, course_data, percent=0, letter_grade=None, passed=False, force_update_subsections=False): + def __init__(self, user, course_data, percent=0.0, letter_grade=None, passed=False, force_update_subsections=False): self.user = user self.course_data = course_data @@ -137,8 +138,7 @@ class CourseGradeBase(object): """ Returns the result from the course grader. """ - course = self.course_data.course - course.set_grading_policy(course.grading_policy) + course = self._prep_course_for_grading() return course.grader.grade( self.graded_subsections_by_format, generate_random_scores=settings.GENERATE_PROFILE_SCORES, @@ -156,6 +156,27 @@ class CourseGradeBase(object): grade_summary['grade'] = self.letter_grade return grade_summary + def _prep_course_for_grading(self): + """ + Make sure any overrides to the grading policy are used. + This is most relevant for CCX courses. + + Right now, we still access the grading policy from the course + object. Once we get the grading policy from the BlockStructure + this will no longer be needed - since BlockStructure correctly + retrieves/uses all field overrides. + """ + course = self.course_data.course + if isinstance(self.course_data.course_key, CCXLocator): + # clean out any field values that may have been set from the + # parent course of the CCX course. + course._field_data_cache = {} # pylint: disable=protected-access + + # this is "magic" code that automatically retrieves any overrides + # to the grading policy and updates the course object. + course.set_grading_policy(course.grading_policy) + return course + def _get_chapter_grade_info(self, chapter, course_structure): """ Helper that returns a dictionary of chapter grade information. @@ -212,6 +233,7 @@ class CourseGrade(CourseGradeBase): self.percent = self._compute_percent(self.grader_result) self.letter_grade = self._compute_letter_grade(grade_cutoffs, self.percent) self.passed = self._compute_passed(grade_cutoffs, self.percent) + return self @lazy def attempted(self): @@ -226,10 +248,10 @@ class CourseGrade(CourseGradeBase): return False def _get_subsection_grade(self, subsection): - # Pass read_only here so the subsection grades can be persisted in bulk at the end. if self.force_update_subsections: return self._subsection_grade_factory.update(subsection) else: + # Pass read_only here so the subsection grades can be persisted in bulk at the end. return self._subsection_grade_factory.create(subsection, read_only=True) @staticmethod diff --git a/lms/djangoapps/grades/course_grade_factory.py b/lms/djangoapps/grades/course_grade_factory.py index 2d1c2daa2a..c239bb5c1c 100644 --- a/lms/djangoapps/grades/course_grade_factory.py +++ b/lms/djangoapps/grades/course_grade_factory.py @@ -20,48 +20,33 @@ class CourseGradeFactory(object): """ GradeResult = namedtuple('GradeResult', ['student', 'course_grade', 'error']) - def create(self, user, course=None, collected_block_structure=None, course_structure=None, course_key=None): + def read( + self, + user, + course=None, + collected_block_structure=None, + course_structure=None, + course_key=None, + create_if_needed=True, + ): """ Returns the CourseGrade for the given user in the course. - Reads the value from storage and validates that the grading - policy hasn't changed since the grade was last computed. + Reads the value from storage. If not in storage, returns a ZeroGrade if ASSUME_ZERO_GRADE_IF_ABSENT. - Else, if changed or not in storage, computes and returns a new value. + Else if create_if_needed, computes and returns a new value. + Else, returns None. At least one of course, collected_block_structure, course_structure, or course_key should be provided. """ course_data = CourseData(user, course, collected_block_structure, course_structure, course_key) try: - course_grade, read_policy_hash = self._read(user, course_data) - if read_policy_hash == course_data.grading_policy_hash: - return course_grade - read_only = False # update the persisted grade since the policy changed; TODO(TNL-6786) remove soon - except PersistentCourseGrade.DoesNotExist: - if assume_zero_if_absent(course_data.course_key): - return self._create_zero(user, course_data) - read_only = True # keep the grade un-persisted; TODO(TNL-6786) remove once all grades are backfilled - - return self._update(user, course_data, read_only) - - def read(self, user, course=None, collected_block_structure=None, course_structure=None, course_key=None): - """ - Returns the CourseGrade for the given user in the course as - persisted in storage. Does NOT verify whether the grading - policy is still valid since the grade was last computed. - If not in storage, returns a ZeroGrade if ASSUME_ZERO_GRADE_IF_ABSENT - else returns None. - - At least one of course, collected_block_structure, course_structure, - or course_key should be provided. - """ - course_data = CourseData(user, course, collected_block_structure, course_structure, course_key) - try: - course_grade, _ = self._read(user, course_data) - return course_grade + return self._read(user, course_data) except PersistentCourseGrade.DoesNotExist: if assume_zero_if_absent(course_data.course_key): return self._create_zero(user, course_data) + elif create_if_needed: + return self._update(user, course_data) else: return None @@ -82,15 +67,7 @@ class CourseGradeFactory(object): or course_key should be provided. """ course_data = CourseData(user, course, collected_block_structure, course_structure, course_key) - return self._update(user, course_data, read_only=False, force_update_subsections=force_update_subsections) - - @contextmanager - def _course_transaction(self, course_key): - """ - Provides a transaction context in which GradeResults are created. - """ - yield - VisibleBlocks.clear_cache(course_key) + return self._update(user, course_data, force_update_subsections=force_update_subsections) def iter( self, @@ -123,6 +100,14 @@ class CourseGradeFactory(object): with dog_stats_api.timer('lms.grades.CourseGradeFactory.iter', tags=stats_tags): yield self._iter_grade_result(user, course_data, force_update) + @contextmanager + def _course_transaction(self, course_key): + """ + Provides a transaction context in which GradeResults are created. + """ + yield + VisibleBlocks.clear_cache(course_key) + def _iter_grade_result(self, user, course_data, force_update): try: kwargs = { @@ -134,7 +119,7 @@ class CourseGradeFactory(object): if force_update: kwargs['force_update_subsections'] = True - method = CourseGradeFactory().update if force_update else CourseGradeFactory().create + method = CourseGradeFactory().update if force_update else CourseGradeFactory().read course_grade = method(**kwargs) return self.GradeResult(user, course_grade, None) except Exception as exc: # pylint: disable=broad-except @@ -166,7 +151,9 @@ class CourseGradeFactory(object): raise PersistentCourseGrade.DoesNotExist persistent_grade = PersistentCourseGrade.read(user.id, course_data.course_key) - course_grade = CourseGrade( + log.debug(u'Grades: Read, %s, User: %s, %s', unicode(course_data), user.id, persistent_grade) + + return CourseGrade( user, course_data, persistent_grade.percent_grade, @@ -174,12 +161,8 @@ class CourseGradeFactory(object): persistent_grade.passed_timestamp is not None, ) - log.debug(u'Grades: Read, %s, User: %s, %s', unicode(course_data), user.id, persistent_grade) - - return course_grade, persistent_grade.grading_policy_hash - @staticmethod - def _update(user, course_data, read_only, force_update_subsections=False): + def _update(user, course_data, force_update_subsections=False): """ Computes, saves, and returns a CourseGrade object for the given user and course. @@ -187,10 +170,9 @@ class CourseGradeFactory(object): COURSE_GRADE_NOW_PASSED if learner has passed course. """ course_grade = CourseGrade(user, course_data, force_update_subsections=force_update_subsections) - course_grade.update() + course_grade = course_grade.update() should_persist = ( - (not read_only) and # TODO(TNL-6786) Remove the read_only boolean once all grades are back-filled. should_persist_grades(course_data.course_key) and course_grade.attempted ) diff --git a/lms/djangoapps/grades/signals/handlers.py b/lms/djangoapps/grades/signals/handlers.py index 9413dc9f6d..b6b9f2b0c3 100644 --- a/lms/djangoapps/grades/signals/handlers.py +++ b/lms/djangoapps/grades/signals/handlers.py @@ -254,7 +254,8 @@ def force_recalculate_course_and_subsection_grades(sender, user, course_key, **k Updates a saved course grade, forcing the subsection grades from which it is calculated to update along the way. """ - if CourseGradeFactory().read(user, course_key=course_key): + previous_course_grade = CourseGradeFactory().read(user, course_key=course_key) + if previous_course_grade and previous_course_grade.attempted: CourseGradeFactory().update(user=user, course_key=course_key, force_update_subsections=True) diff --git a/lms/djangoapps/grades/tests/test_grades.py b/lms/djangoapps/grades/tests/test_grades.py index 2b6769a98e..ca253031ea 100644 --- a/lms/djangoapps/grades/tests/test_grades.py +++ b/lms/djangoapps/grades/tests/test_grades.py @@ -79,7 +79,7 @@ class TestGradeIteration(SharedModuleStoreTestCase): self.assertIsNone(course_grade.letter_grade) self.assertEqual(course_grade.percent, 0.0) - @patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.create') + @patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') def test_grading_exception(self, mock_course_grade): """Test that we correctly capture exception messages that bubble up from grading. Note that we only see errors at this level if the grading @@ -287,7 +287,7 @@ class TestScoreForModule(SharedModuleStoreTestCase): answer_problem(cls.course, cls.request, cls.l, score=1, max_value=3) answer_problem(cls.course, cls.request, cls.n, score=3, max_value=10) - cls.course_grade = CourseGradeFactory().create(cls.request.user, cls.course) + cls.course_grade = CourseGradeFactory().read(cls.request.user, cls.course) def test_score_chapter(self): earned, possible = self.course_grade.score_for_module(self.a.location) diff --git a/lms/djangoapps/grades/tests/test_new.py b/lms/djangoapps/grades/tests/test_new.py index 103ab37eb2..6c8e35e4d1 100644 --- a/lms/djangoapps/grades/tests/test_new.py +++ b/lms/djangoapps/grades/tests/test_new.py @@ -97,12 +97,12 @@ class GradeTestBase(SharedModuleStoreTestCase): super(GradeTestBase, self).setUp() self.request = get_mock_request(UserFactory()) self.client.login(username=self.request.user.username, password="test") - self._update_grading_policy() + self._set_grading_policy() self.course_structure = get_course_blocks(self.request.user, self.course.location) self.subsection_grade_factory = SubsectionGradeFactory(self.request.user, self.course, self.course_structure) CourseEnrollment.enroll(self.request.user, self.course.id) - def _update_grading_policy(self, passing=0.5): + def _set_grading_policy(self, passing=0.5): """ Updates the course's grading policy. """ @@ -149,7 +149,7 @@ class TestCourseGradeFactory(GradeTestBase): self.assertEqual(access.error_code, 'not_visible_to_user') # with self.assertNoExceptionRaised: <- this isn't a real method, it's an implicit assumption - grade = CourseGradeFactory().create(self.request.user, invisible_course) + grade = CourseGradeFactory().read(self.request.user, invisible_course) self.assertEqual(grade.percent, 0) @patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) @@ -171,93 +171,59 @@ class TestCourseGradeFactory(GradeTestBase): enabled_for_course=course_setting ): with patch('lms.djangoapps.grades.models.PersistentCourseGrade.read') as mock_read_grade: - grade_factory.create(self.request.user, self.course) + grade_factory.read(self.request.user, self.course) self.assertEqual(mock_read_grade.called, feature_flag and course_setting) - def test_create(self): - grade_factory = CourseGradeFactory() - - def _assert_create(expected_pass): - """ - Creates the grade, ensuring it is as expected. - """ - course_grade = grade_factory.create(self.request.user, self.course) - self.assertEqual(course_grade.letter_grade, u'Pass' if expected_pass else None) - self.assertEqual(course_grade.percent, 0.5) - - with self.assertNumQueries(13), mock_get_score(1, 2): - _assert_create(expected_pass=True) - - with self.assertNumQueries(13), mock_get_score(1, 2): - grade_factory.update(self.request.user, self.course) - - with self.assertNumQueries(1): - _assert_create(expected_pass=True) - - self._update_grading_policy(passing=0.9) - - with self.assertNumQueries(8): - _assert_create(expected_pass=False) - - @ddt.data(True, False) - def test_create_zero(self, assume_zero_enabled): - with waffle().override(ASSUME_ZERO_GRADE_IF_ABSENT, active=assume_zero_enabled): - grade_factory = CourseGradeFactory() - course_grade = grade_factory.create(self.request.user, self.course) - self._assert_zero_grade(course_grade, ZeroCourseGrade if assume_zero_enabled else CourseGrade) - - def test_create_zero_subs_grade_for_nonzero_course_grade(self): - with waffle().override(ASSUME_ZERO_GRADE_IF_ABSENT): - subsection = self.course_structure[self.sequence.location] - with mock_get_score(1, 2): - self.subsection_grade_factory.update(subsection) - course_grade = CourseGradeFactory().update(self.request.user, self.course) - subsection1_grade = course_grade.subsection_grades[self.sequence.location] - subsection2_grade = course_grade.subsection_grades[self.sequence2.location] - self.assertIsInstance(subsection1_grade, SubsectionGrade) - self.assertIsInstance(subsection2_grade, ZeroSubsectionGrade) - def test_read(self): grade_factory = CourseGradeFactory() - with mock_get_score(1, 2): - grade_factory.update(self.request.user, self.course) - def _assert_read(): + def _assert_read(expected_pass, expected_percent): """ - Reads the grade, ensuring it is as expected and requires just one query + Creates the grade, ensuring it is as expected. """ - with self.assertNumQueries(1): - course_grade = grade_factory.read(self.request.user, self.course) - self.assertEqual(course_grade.letter_grade, u'Pass') - self.assertEqual(course_grade.percent, 0.5) + course_grade = grade_factory.read(self.request.user, self.course) + self.assertEqual(course_grade.letter_grade, u'Pass' if expected_pass else None) + self.assertEqual(course_grade.percent, expected_percent) - _assert_read() - self._update_grading_policy(passing=0.9) - _assert_read() + with self.assertNumQueries(1), mock_get_score(1, 2): + _assert_read(expected_pass=False, expected_percent=0) - @ddt.data(True, False) - def test_read_zero(self, assume_zero_enabled): + with self.assertNumQueries(37), mock_get_score(1, 2): + grade_factory.update(self.request.user, self.course, force_update_subsections=True) + + with self.assertNumQueries(1): + _assert_read(expected_pass=True, expected_percent=0.5) + + @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) + @ddt.data(*itertools.product((True, False), (True, False))) + @ddt.unpack + def test_read_zero(self, assume_zero_enabled, create_if_needed): with waffle().override(ASSUME_ZERO_GRADE_IF_ABSENT, active=assume_zero_enabled): grade_factory = CourseGradeFactory() - course_grade = grade_factory.read(self.request.user, course_key=self.course.id) - if assume_zero_enabled: - self._assert_zero_grade(course_grade, ZeroCourseGrade) + course_grade = grade_factory.read(self.request.user, self.course, create_if_needed=create_if_needed) + if create_if_needed or assume_zero_enabled: + self._assert_zero_grade(course_grade, ZeroCourseGrade if assume_zero_enabled else CourseGrade) else: self.assertIsNone(course_grade) + def test_create_zero_subs_grade_for_nonzero_course_grade(self): + subsection = self.course_structure[self.sequence.location] + with mock_get_score(1, 2): + self.subsection_grade_factory.update(subsection) + course_grade = CourseGradeFactory().update(self.request.user, self.course) + subsection1_grade = course_grade.subsection_grades[self.sequence.location] + subsection2_grade = course_grade.subsection_grades[self.sequence2.location] + self.assertIsInstance(subsection1_grade, SubsectionGrade) + self.assertIsInstance(subsection2_grade, ZeroSubsectionGrade) + @ddt.data(True, False) def test_iter_force_update(self, force_update): - base_string = 'lms.djangoapps.grades.subsection_grade_factory.SubsectionGradeFactory.{}' - desired_method_name = base_string.format('update' if force_update else 'create') - undesired_method_name = base_string.format('create' if force_update else 'update') - with patch(desired_method_name) as desired_call: - with patch(undesired_method_name) as undesired_call: - set(CourseGradeFactory().iter( - users=[self.request.user], course=self.course, force_update=force_update - )) + with patch('lms.djangoapps.grades.subsection_grade_factory.SubsectionGradeFactory.update') as mock_update: + set(CourseGradeFactory().iter( + users=[self.request.user], course=self.course, force_update=force_update + )) - self.assertTrue(desired_call.called) - self.assertFalse(undesired_call.called) + self.assertEqual(mock_update.called, force_update) def test_course_grade_summary(self): with mock_get_score(1, 2): @@ -322,45 +288,13 @@ class TestSubsectionGradeFactory(ProblemSubmissionTestMixin, GradeTestBase): (expected_earned, expected_possible), ) - def test_create(self): + def test_create_zero(self): """ - Assuming the underlying score reporting methods work, - test that the score is calculated properly. + Test that a zero grade is returned. """ - with mock_get_score(1, 2): - grade = self.subsection_grade_factory.create(self.sequence) - self.assert_grade(grade, 1, 2) - - def test_create_internals(self): - """ - Tests to ensure that a persistent subsection grade is - created, saved, then fetched on re-request. - """ - with patch( - 'lms.djangoapps.grades.subsection_grade.PersistentSubsectionGrade.create_grade', - wraps=PersistentSubsectionGrade.create_grade - ) as mock_create_grade: - with patch( - 'lms.djangoapps.grades.subsection_grade_factory.SubsectionGradeFactory._get_bulk_cached_grade', - wraps=self.subsection_grade_factory._get_bulk_cached_grade - ) as mock_get_bulk_cached_grade: - with self.assertNumQueries(14): - with mock_get_score(1, 2): - grade_a = self.subsection_grade_factory.create(self.sequence) - self.assertTrue(mock_get_bulk_cached_grade.called) - self.assertTrue(mock_create_grade.called) - - mock_get_bulk_cached_grade.reset_mock() - mock_create_grade.reset_mock() - - with self.assertNumQueries(1): - grade_b = self.subsection_grade_factory.create(self.sequence) - self.assertTrue(mock_get_bulk_cached_grade.called) - self.assertFalse(mock_create_grade.called) - - self.assertEqual(grade_a.url_name, grade_b.url_name) - grade_b.all_total.first_attempted = grade_a.all_total.first_attempted = None - self.assertEqual(grade_a.all_total, grade_b.all_total) + grade = self.subsection_grade_factory.create(self.sequence) + self.assertIsInstance(grade, ZeroSubsectionGrade) + self.assert_grade(grade, 0.0, 1.0) def test_update(self): """ @@ -426,6 +360,7 @@ class TestSubsectionGradeFactory(ProblemSubmissionTestMixin, GradeTestBase): self.assertEqual(mock_read_saved_grade.called, feature_flag and course_setting) +@patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) @ddt.ddt class ZeroGradeTest(GradeTestBase): """ @@ -762,40 +697,3 @@ class TestCourseGradeLogging(ProblemSubmissionTestMixin, SharedModuleStoreTestCa self.course_structure = get_course_blocks(self.request.user, self.course.location) self.subsection_grade_factory = SubsectionGradeFactory(self.request.user, self.course, self.course_structure) CourseEnrollment.enroll(self.request.user, self.course.id) - - def _create_course_grade_and_check_logging( - self, - factory_method, - log_mock, - log_statement, - ): - """ - Creates a course grade and asserts that the associated logging - matches the expected totals passed in to the function. - """ - factory_method(self.request.user, self.course) - self.assertIn(log_statement, log_mock.call_args[0][0]) - self.assertIn(unicode(self.course.id), log_mock.call_args[0][1]) - self.assertEquals(self.request.user.id, log_mock.call_args[0][2]) - - def test_course_grade_logging(self): - grade_factory = CourseGradeFactory() - with persistent_grades_feature_flags( - global_flag=True, - enabled_for_all_courses=False, - course_id=self.course.id, - enabled_for_course=True - ): - with patch('lms.djangoapps.grades.course_grade_factory.log') as log_mock: - with mock_get_score(1, 2): - # read, but not persisted - self._create_course_grade_and_check_logging(grade_factory.create, log_mock.info, u'Update') - - # update and persist - self._create_course_grade_and_check_logging(grade_factory.update, log_mock.info, u'Update') - - # read from persistence, using create - self._create_course_grade_and_check_logging(grade_factory.create, log_mock.debug, u'Read') - - # read from persistence, using read - self._create_course_grade_and_check_logging(grade_factory.read, log_mock.debug, u'Read') diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index dd3ea4a1f9..0848d5c15b 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -164,9 +164,9 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEquals(mock_block_structure_create.call_count, 1) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 31, True), + (ModuleStoreEnum.Type.mongo, 1, 27, True), (ModuleStoreEnum.Type.mongo, 1, 27, False), - (ModuleStoreEnum.Type.split, 3, 31, True), + (ModuleStoreEnum.Type.split, 3, 27, True), (ModuleStoreEnum.Type.split, 3, 27, False), ) @ddt.unpack @@ -179,8 +179,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self._apply_recalculate_subsection_grade() @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 31), - (ModuleStoreEnum.Type.split, 3, 31), + (ModuleStoreEnum.Type.mongo, 1, 27), + (ModuleStoreEnum.Type.split, 3, 27), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): diff --git a/lms/djangoapps/grades/tests/utils.py b/lms/djangoapps/grades/tests/utils.py index 5d6e06c35b..4842826700 100644 --- a/lms/djangoapps/grades/tests/utils.py +++ b/lms/djangoapps/grades/tests/utils.py @@ -12,17 +12,21 @@ from xmodule.graders import ProblemScore @contextmanager -def mock_passing_grade(grade_pass='Pass', percent=0.75, ): +def mock_passing_grade(letter_grade='Pass', percent=0.75, ): """ Mock the grading function to always return a passing grade. """ - with patch('lms.djangoapps.grades.course_grade.CourseGrade._compute_letter_grade') as mock_letter_grade: - with patch('lms.djangoapps.grades.course_grade.CourseGrade._compute_percent') as mock_percent_grade: - with patch('lms.djangoapps.grades.course_grade.CourseGrade.attempted') as mock_attempted: - mock_letter_grade.return_value = grade_pass - mock_percent_grade.return_value = percent - mock_attempted.return_value = True - yield + passing_grade_fields = dict( + letter_grade=letter_grade, + percent=percent, + passed=letter_grade is not None, + attempted=True, + ) + with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_grade_read: + mock_grade_read.return_value = MagicMock(**passing_grade_fields) + with patch('lms.djangoapps.grades.course_grade.CourseGrade.update') as mock_grade_update: + mock_grade_update.return_value = MagicMock(**passing_grade_fields) + yield @contextmanager diff --git a/lms/djangoapps/instructor/tests/test_spoc_gradebook.py b/lms/djangoapps/instructor/tests/test_spoc_gradebook.py index ade14980ae..a0a50a44c8 100644 --- a/lms/djangoapps/instructor/tests/test_spoc_gradebook.py +++ b/lms/djangoapps/instructor/tests/test_spoc_gradebook.py @@ -7,6 +7,7 @@ from nose.plugins.attrib import attr from capa.tests.response_xml_factory import StringResponseXMLFactory from courseware.tests.factories import StudentModuleFactory +from lms.djangoapps.grades.tasks import compute_all_grades_for_course from student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -73,6 +74,7 @@ class TestGradebook(SharedModuleStoreTestCase): course_id=self.course.id, module_state_key=item.location ) + compute_all_grades_for_course.apply_async(kwargs={'course_key': unicode(self.course.id)}) self.response = self.client.get(reverse( 'spoc_gradebook', diff --git a/lms/djangoapps/instructor/views/gradebook_api.py b/lms/djangoapps/instructor/views/gradebook_api.py index 46de7caded..aa690eddda 100644 --- a/lms/djangoapps/instructor/views/gradebook_api.py +++ b/lms/djangoapps/instructor/views/gradebook_api.py @@ -89,7 +89,7 @@ def get_grade_book_page(request, course, course_key): 'username': student.username, 'id': student.id, 'email': student.email, - 'grade_summary': CourseGradeFactory().create(student, course).summary + 'grade_summary': CourseGradeFactory().read(student, course).summary } for student in enrolled_students ] diff --git a/lms/djangoapps/instructor_task/tests/test_integration.py b/lms/djangoapps/instructor_task/tests/test_integration.py index 419f764eb4..38f93a03da 100644 --- a/lms/djangoapps/instructor_task/tests/test_integration.py +++ b/lms/djangoapps/instructor_task/tests/test_integration.py @@ -131,7 +131,7 @@ class TestRescoringTask(TestIntegrationTask): # are in sync. expected_subsection_grade = expected_score - course_grade = CourseGradeFactory().create(user, self.course) + course_grade = CourseGradeFactory().read(user, self.course) self.assertEquals( course_grade.graded_subsections_by_format['Homework'][self.problem_section.location].graded_total.earned, expected_subsection_grade, diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index bb64881cc8..a0d1021b43 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -395,7 +395,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase): RequestCache.clear_request_cache() - expected_query_count = 41 + expected_query_count = 36 with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'): with check_mongo_calls(mongo_count): with self.assertNumQueries(expected_query_count): @@ -1976,7 +1976,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): 'failed': 3, 'skipped': 2 } - with self.assertNumQueries(176): + with self.assertNumQueries(106): self.assertCertificatesGenerated(task_input, expected_results) expected_results = { diff --git a/lms/djangoapps/lti_provider/tasks.py b/lms/djangoapps/lti_provider/tasks.py index 837145d367..549c38afb4 100644 --- a/lms/djangoapps/lti_provider/tasks.py +++ b/lms/djangoapps/lti_provider/tasks.py @@ -110,7 +110,7 @@ def send_composite_outcome(user_id, course_id, assignment_id, version): mapped_usage_key = assignment.usage_key.map_into_course(course_key) user = User.objects.get(id=user_id) course = modulestore().get_course(course_key, depth=0) - course_grade = CourseGradeFactory().create(user, course) + course_grade = CourseGradeFactory().read(user, course) earned, possible = course_grade.score_for_module(mapped_usage_key) if possible == 0: weighted_score = 0 diff --git a/lms/djangoapps/lti_provider/tests/test_tasks.py b/lms/djangoapps/lti_provider/tests/test_tasks.py index 2f25af26fe..d94dfa4c1e 100644 --- a/lms/djangoapps/lti_provider/tests/test_tasks.py +++ b/lms/djangoapps/lti_provider/tests/test_tasks.py @@ -101,7 +101,7 @@ class SendCompositeOutcomeTest(BaseOutcomeTest): ) self.course_grade = MagicMock() self.course_grade_mock = self.setup_patch( - 'lti_provider.tasks.CourseGradeFactory.create', self.course_grade + 'lti_provider.tasks.CourseGradeFactory.read', self.course_grade ) self.module_store = MagicMock() self.module_store.get_item = MagicMock(return_value=self.descriptor) diff --git a/lms/djangoapps/shoppingcart/tests/test_models.py b/lms/djangoapps/shoppingcart/tests/test_models.py index 94597e42b1..4180ad4b95 100644 --- a/lms/djangoapps/shoppingcart/tests/test_models.py +++ b/lms/djangoapps/shoppingcart/tests/test_models.py @@ -247,12 +247,8 @@ class OrderTest(ModuleStoreTestCase): self.assertEqual(cart.status, status) self.assertEqual(item.status, status) - @override_settings( - LMS_SEGMENT_KEY="foobar", - FEATURES={ - 'STORE_BILLING_INFO': True, - } - ) + @override_settings(LMS_SEGMENT_KEY="foobar") + @patch.dict(settings.FEATURES, {'STORE_BILLING_INFO': True}) def test_purchase(self): # This test is for testing the subclassing functionality of OrderItem, but in # order to do this, we end up testing the specific functionality of @@ -914,12 +910,8 @@ class CertificateItemTest(ModuleStoreTestCase): cert_item = CertificateItem.add_to_order(cart, self.course_key, self.cost, 'honor') self.assertEquals(cert_item.single_item_receipt_template, 'shoppingcart/receipt.html') - @override_settings( - LMS_SEGMENT_KEY="foobar", - FEATURES={ - 'STORE_BILLING_INFO': True, - } - ) + @override_settings(LMS_SEGMENT_KEY="foobar") + @patch.dict(settings.FEATURES, {'STORE_BILLING_INFO': True}) @patch('student.models.CourseEnrollment.refund_cutoff_date') def test_refund_cert_callback_no_expiration(self, cutoff_date): # When there is no expiration date on a verified mode, the user can always get a refund @@ -956,12 +948,8 @@ class CertificateItemTest(ModuleStoreTestCase): self.assertFalse(target_certs[0].refund_requested_time) self.assertEquals(target_certs[0].order.status, 'purchased') - @override_settings( - LMS_SEGMENT_KEY="foobar", - FEATURES={ - 'STORE_BILLING_INFO': True, - } - ) + @override_settings(LMS_SEGMENT_KEY="foobar") + @patch.dict(settings.FEATURES, {'STORE_BILLING_INFO': True}) @patch('student.models.CourseEnrollment.refund_cutoff_date') def test_refund_cert_callback_before_expiration(self, cutoff_date): # If the expiration date has not yet passed on a verified mode, the user can be refunded diff --git a/lms/envs/bok_choy.py b/lms/envs/bok_choy.py index 461907c34e..677c475566 100644 --- a/lms/envs/bok_choy.py +++ b/lms/envs/bok_choy.py @@ -89,13 +89,17 @@ BLOCK_STRUCTURES_SETTINGS = dict( TASK_DEFAULT_RETRY_DELAY=0, ) -###################### Grade Downloads ###################### +###################### Grades ###################### GRADES_DOWNLOAD = { 'STORAGE_TYPE': 'localfs', 'BUCKET': 'edx-grades', 'ROOT_PATH': os.path.join(mkdtemp(), 'edx-s3', 'grades'), } +FEATURES['PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS'] = True +FEATURES['ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS'] = True + + # Configure the LMS to use our stub XQueue implementation XQUEUE_INTERFACE['url'] = 'http://localhost:8040' diff --git a/lms/envs/test.py b/lms/envs/test.py index 21cabfd62d..2a9268ae22 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -307,6 +307,7 @@ FEATURES['ENABLE_VIDEO_ABSTRACTION_LAYER_API'] = True ########################### Grades ################################# FEATURES['PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS'] = True +FEATURES['ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS'] = True ###################### Payment ##############################3 # Enable fake payment processing page