Merge pull request #13881 from edx/efischer/tnl-5314

Dashboard Final Grade Updates

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(
@@ -106,6 +111,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'

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
 

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
+
+        return CourseGrade.get_persisted_grade(self.student, course)
 
-    def _get_saved_grade(self, course, course_structure):  # pylint: disable=unused-argument
+    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

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)