diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index adefb22f0f..49dd182c2d 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -19,7 +19,7 @@ import httpretty from markupsafe import escape from mock import Mock, patch from nose.plugins.attrib import attr -from opaque_keys.edx.locations import SlashSeparatedCourseKey +from opaque_keys.edx.locations import SlashSeparatedCourseKey, CourseLocator from provider.constants import CONFIDENTIAL from pyquery import PyQuery as pq import pytz @@ -58,6 +58,7 @@ from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls log = logging.getLogger(__name__) +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @ddt.ddt class CourseEndingTest(TestCase): """Test things related to course endings: certificates, surveys, etc""" @@ -75,9 +76,13 @@ class CourseEndingTest(TestCase): @patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': False}) def test_cert_info(self): - user = Mock(username="fred") + user = Mock(username="fred", id="1") survey_url = "http://a_survey.com" - course = Mock(end_of_course_survey_url=survey_url, certificates_display_behavior='end') + course = Mock( + end_of_course_survey_url=survey_url, + certificates_display_behavior='end', + id=CourseLocator(org="x", course="y", run="z"), + ) course_mode = 'honor' self.assertEqual( @@ -105,6 +110,24 @@ class CourseEndingTest(TestCase): } ) + cert_status = {'status': 'generating', 'grade': '67', 'mode': 'honor'} + with patch('lms.djangoapps.grades.new.course_grade.CourseGradeFactory.get_persisted') as patch_persisted_grade: + patch_persisted_grade.return_value = Mock(percent=100) + self.assertEqual( + _cert_info(user, course, cert_status, course_mode), + { + 'status': 'generating', + 'show_disabled_download_button': True, + 'show_download_url': False, + 'show_survey_button': True, + 'survey_url': survey_url, + 'grade': '100', + 'mode': 'honor', + 'linked_in_url': None, + 'can_unenroll': False, + } + ) + cert_status = {'status': 'generating', 'grade': '67', 'mode': 'honor'} self.assertEqual( _cert_info(user, course, cert_status, course_mode), @@ -165,7 +188,7 @@ class CourseEndingTest(TestCase): ) # Test a course that doesn't have a survey specified - course2 = Mock(end_of_course_survey_url=None) + course2 = Mock(end_of_course_survey_url=None, id=CourseLocator(org="a", course="b", run="c")) cert_status = { 'status': 'notpassing', 'grade': '67', 'download_url': download_url, 'mode': 'honor' diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index e1db838b65..aa0c0ce42f 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -65,6 +65,7 @@ from certificates.api import ( # pylint: disable=import-error get_certificate_url, has_html_certificates_enabled, ) +from lms.djangoapps.grades.new.course_grade import CourseGradeFactory from xmodule.modulestore.django import modulestore from opaque_keys import InvalidKeyError @@ -402,14 +403,17 @@ def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disa cert_status['download_url'] ) - if status in ('generating', 'ready', 'notpassing', 'restricted', 'auditing', 'unverified'): - if 'grade' not in cert_status: + if status in {'generating', 'ready', 'notpassing', 'restricted', 'auditing', 'unverified'}: + persisted_grade = CourseGradeFactory(user).get_persisted(course_overview) + if persisted_grade is not None: + status_dict['grade'] = unicode(persisted_grade.percent) + elif 'grade' in cert_status: + status_dict['grade'] = cert_status['grade'] + else: # Note: as of 11/20/2012, we know there are students in this state-- cs169.1x, # who need to be regraded (we weren't tracking 'notpassing' at first). # We can add a log.warning here once we think it shouldn't happen. return default_info - else: - status_dict['grade'] = cert_status['grade'] return status_dict diff --git a/lms/djangoapps/grades/new/course_grade.py b/lms/djangoapps/grades/new/course_grade.py index 261971f16d..8efef24382 100644 --- a/lms/djangoapps/grades/new/course_grade.py +++ b/lms/djangoapps/grades/new/course_grade.py @@ -240,6 +240,25 @@ class CourseGrade(object): return course_grade + @classmethod + def get_persisted_grade(cls, user, course): + """ + Gets the persisted grade in the database, without checking + whether it is up-to-date with the course's grading policy. + For read use only. + """ + try: + persistent_grade = PersistentCourseGrade.read_course_grade(user.id, course.id) + except PersistentCourseGrade.DoesNotExist: + return None + else: + course_grade = CourseGrade(user, course, None) # no course structure needed + course_grade._percent = persistent_grade.percent_grade # pylint: disable=protected-access + course_grade._letter_grade = persistent_grade.letter_grade # pylint: disable=protected-access + course_grade.course_version = persistent_grade.course_version + course_grade.course_edited_timestamp = persistent_grade.course_edited_timestamp + return course_grade + @staticmethod def _calc_percent(grade_value): """ @@ -328,17 +347,17 @@ class CourseGradeFactory(object): course_structure = get_course_blocks(self.student, course.location) self._compute_and_update_grade(course, course_structure) - def _compute_and_update_grade(self, course, course_structure, read_only=False): + def get_persisted(self, course): """ - Freshly computes and updates the grade for the student and course. - - If read_only is True, doesn't save any updates to the grades. + Returns the saved grade for the given course and student, + irrespective of whether the saved grade is up-to-date. """ - course_grade = CourseGrade(self.student, course, course_structure) - course_grade.compute_and_update(read_only) - return course_grade + if not PersistentGradesEnabledFlag.feature_enabled(course.id): + return None - def _get_saved_grade(self, course, course_structure): # pylint: disable=unused-argument + return CourseGrade.get_persisted_grade(self.student, course) + + def _get_saved_grade(self, course, course_structure): """ Returns the saved grade for the given course and student. """ @@ -351,6 +370,16 @@ class CourseGradeFactory(object): course_structure ) + def _compute_and_update_grade(self, course, course_structure, read_only=False): + """ + Freshly computes and updates the grade for the student and course. + + If read_only is True, doesn't save any updates to the grades. + """ + course_grade = CourseGrade(self.student, course, course_structure) + course_grade.compute_and_update(read_only) + return course_grade + def _user_has_access_to_course(self, course_structure): """ Given a course structure, returns whether the user diff --git a/lms/djangoapps/grades/tests/test_new.py b/lms/djangoapps/grades/tests/test_new.py index 17811ee458..0797522769 100644 --- a/lms/djangoapps/grades/tests/test_new.py +++ b/lms/djangoapps/grades/tests/test_new.py @@ -82,6 +82,23 @@ class TestCourseGradeFactory(GradeTestBase): """ Test that CourseGrades are calculated properly """ + def setUp(self): + super(TestCourseGradeFactory, self).setUp() + grading_policy = { + "GRADER": [ + { + "type": "Homework", + "min_count": 1, + "drop_count": 0, + "short_label": "HW", + "weight": 1.0, + }, + ], + "GRADE_CUTOFFS": { + "Pass": 0.5, + }, + } + self.course.set_grading_policy(grading_policy) @patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) @ddt.data( @@ -106,7 +123,26 @@ class TestCourseGradeFactory(GradeTestBase): self.assertEqual(mock_save_grades.called, feature_flag and course_setting) def test_course_grade_creation(self): - grading_policy = { + grade_factory = CourseGradeFactory(self.request.user) + with mock_get_score(1, 2): + course_grade = grade_factory.create(self.course) + self.assertEqual(course_grade.letter_grade, u'Pass') + self.assertEqual(course_grade.percent, 0.5) + + def test_get_persisted(self): + grade_factory = CourseGradeFactory(self.request.user) + # first, create a grade in the database + with mock_get_score(1, 2): + grade_factory.create(self.course, read_only=False) + + # retrieve the grade, ensuring it is as expected and take just one query + with self.assertNumQueries(1): + course_grade = grade_factory.get_persisted(self.course) + self.assertEqual(course_grade.letter_grade, u'Pass') + self.assertEqual(course_grade.percent, 0.5) + + # update the grading policy + new_grading_policy = { "GRADER": [ { "type": "Homework", @@ -117,13 +153,15 @@ class TestCourseGradeFactory(GradeTestBase): }, ], "GRADE_CUTOFFS": { - "Pass": 0.5, + "Pass": 0.9, }, } - self.course.set_grading_policy(grading_policy) - grade_factory = CourseGradeFactory(self.request.user) - with mock_get_score(1, 2): - course_grade = grade_factory.create(self.course) + self.course.set_grading_policy(new_grading_policy) + + # ensure the grade can still be retrieved via get_persisted + # despite its outdated grading policy + with self.assertNumQueries(1): + course_grade = grade_factory.get_persisted(self.course) self.assertEqual(course_grade.letter_grade, u'Pass') self.assertEqual(course_grade.percent, 0.5) @@ -168,18 +206,18 @@ class TestSubsectionGradeFactory(ProblemSubmissionTestMixin, GradeTestBase): with patch( 'lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory._get_bulk_cached_grade', wraps=self.subsection_grade_factory._get_bulk_cached_grade - ) as mock_get_saved_grade: + ) as mock_get_bulk_cached_grade: with self.assertNumQueries(14): grade_a = self.subsection_grade_factory.create(self.sequence) - self.assertTrue(mock_get_saved_grade.called) + self.assertTrue(mock_get_bulk_cached_grade.called) self.assertTrue(mock_create_grade.called) - mock_get_saved_grade.reset_mock() + mock_get_bulk_cached_grade.reset_mock() mock_create_grade.reset_mock() with self.assertNumQueries(0): grade_b = self.subsection_grade_factory.create(self.sequence) - self.assertTrue(mock_get_saved_grade.called) + self.assertTrue(mock_get_bulk_cached_grade.called) self.assertFalse(mock_create_grade.called) self.assertEqual(grade_a.url_name, grade_b.url_name)