diff --git a/lms/djangoapps/grades/migrations/0007_add_passed_timestamp_column.py b/lms/djangoapps/grades/migrations/0007_add_passed_timestamp_column.py new file mode 100644 index 0000000000..6bc4baffe1 --- /dev/null +++ b/lms/djangoapps/grades/migrations/0007_add_passed_timestamp_column.py @@ -0,0 +1,23 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('grades', '0006_persistent_course_grades'), + ] + + operations = [ + migrations.AddField( + model_name='persistentcoursegrade', + name='passed_timestamp', + field=models.DateTimeField(null=True, verbose_name='Date learner earned a passing grade', blank=True), + ), + migrations.AlterIndexTogether( + name='persistentcoursegrade', + index_together=set([('passed_timestamp', 'course_id')]), + ), + ] diff --git a/lms/djangoapps/grades/models.py b/lms/djangoapps/grades/models.py index 0cb1baf5f9..a870942e8f 100644 --- a/lms/djangoapps/grades/models.py +++ b/lms/djangoapps/grades/models.py @@ -16,6 +16,7 @@ from lazy import lazy import logging from django.db import models +from django.utils.timezone import now from model_utils.models import TimeStampedModel from coursewarehistoryextended.fields import UnsignedBigIntAutoField @@ -378,9 +379,13 @@ class PersistentCourseGrade(TimeStampedModel): # (course_id, user_id) for individual grades # (course_id) for instructors to see all course grades, implicitly created via the unique_together constraint # (user_id) for course dashboard; explicitly declared as an index below + # (passed_timestamp, course_id) for tracking when users first earned a passing grade. unique_together = [ ('course_id', 'user_id'), ] + index_together = [ + ('passed_timestamp', 'course_id'), + ] # primary key will need to be large for this table id = UnsignedBigIntAutoField(primary_key=True) # pylint: disable=invalid-name @@ -396,18 +401,21 @@ class PersistentCourseGrade(TimeStampedModel): percent_grade = models.FloatField(blank=False) letter_grade = models.CharField(u'Letter grade for course', blank=False, max_length=255) + # Information related to course completion + passed_timestamp = models.DateTimeField(u'Date learner earned a passing grade', blank=True, null=True) + def __unicode__(self): """ Returns a string representation of this model. """ - return u"{} user: {}, course version: {}, grading policy: {}, percent grade {}%, letter grade {}".format( - type(self).__name__, - self.user_id, - self.course_version, - self.grading_policy_hash, - self.percent_grade, - self.letter_grade, - ) + return u', '.join([ + u"{} user: {}".format(type(self).__name__, self.user_id), + u"course version: {}".format(self.course_version), + u"grading policy: {}".format(self.grading_policy_hash), + u"percent grade: {}%".format(self.percent_grade), + u"letter grade: {}".format(self.letter_grade), + u"passed_timestamp: {}".format(self.passed_timestamp), + ]) @classmethod def read_course_grade(cls, user_id, course_id): @@ -428,6 +436,7 @@ class PersistentCourseGrade(TimeStampedModel): Creates a course grade in the database. Returns a PersistedCourseGrade object. """ + passed = kwargs.pop('passed') if kwargs.get('course_version', None) is None: kwargs['course_version'] = "" @@ -436,4 +445,7 @@ class PersistentCourseGrade(TimeStampedModel): course_id=course_id, defaults=kwargs ) + if passed and not grade.passed_timestamp: + grade.passed_timestamp = now() + grade.save() return grade diff --git a/lms/djangoapps/grades/new/course_grade.py b/lms/djangoapps/grades/new/course_grade.py index 958f084312..b87be568ba 100644 --- a/lms/djangoapps/grades/new/course_grade.py +++ b/lms/djangoapps/grades/new/course_grade.py @@ -3,10 +3,12 @@ CourseGrade Class """ from collections import defaultdict +from logging import getLogger + from django.conf import settings from django.core.exceptions import PermissionDenied from lazy import lazy -from logging import getLogger + from lms.djangoapps.course_blocks.api import get_course_blocks from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag from openedx.core.djangoapps.signals.signals import GRADES_UPDATED @@ -163,6 +165,7 @@ class CourseGrade(object): grading_policy_hash=grading_policy_hash, percent_grade=self.percent, letter_grade=self.letter_grade or "", + passed=self.passed, ) self._signal_listeners_when_grade_computed() diff --git a/lms/djangoapps/grades/tests/test_models.py b/lms/djangoapps/grades/tests/test_models.py index 60480bef45..37aacdefd6 100644 --- a/lms/djangoapps/grades/tests/test_models.py +++ b/lms/djangoapps/grades/tests/test_models.py @@ -10,6 +10,8 @@ import json from django.db.utils import IntegrityError from django.test import TestCase +from django.utils.timezone import now +from freezegun import freeze_time from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator from lms.djangoapps.grades.models import ( @@ -271,10 +273,11 @@ class PersistentCourseGradesTest(GradesModelTestCase): ), "percent_grade": 77.7, "letter_grade": "Great job", + "passed": True } def test_update(self): - created_grade = PersistentCourseGrade.objects.create(**self.params) + created_grade = PersistentCourseGrade.update_or_create_course_grade(**self.params) self.params["percent_grade"] = 88.8 self.params["letter_grade"] = "Better job" updated_grade = PersistentCourseGrade.update_or_create_course_grade(**self.params) @@ -282,11 +285,61 @@ class PersistentCourseGradesTest(GradesModelTestCase): self.assertEqual(updated_grade.letter_grade, "Better job") self.assertEqual(created_grade.id, updated_grade.id) + def test_passed_timestamp(self): + # When the user has not passed, passed_timestamp is None + self.params.update({ + u'percent_grade': 25.0, + u'letter_grade': u'', + u'passed': False, + }) + grade = PersistentCourseGrade.update_or_create_course_grade(**self.params) + self.assertIsNone(grade.passed_timestamp) + + # After the user earns a passing grade, the passed_timestamp is set + self.params.update({ + u'percent_grade': 75.0, + u'letter_grade': u'C', + u'passed': True, + }) + grade = PersistentCourseGrade.update_or_create_course_grade(**self.params) + passed_timestamp = grade.passed_timestamp + self.assertEqual(grade.letter_grade, u'C') + self.assertIsInstance(passed_timestamp, datetime) + + # After the user improves their score, the new grade is reflected, but + # the passed_timestamp remains the same. + self.params.update({ + u'percent_grade': 95.0, + u'letter_grade': u'A', + u'passed': True, + }) + grade = PersistentCourseGrade.update_or_create_course_grade(**self.params) + self.assertEqual(grade.letter_grade, u'A') + self.assertEqual(grade.passed_timestamp, passed_timestamp) + + # If the grade later reverts to a failing grade, they keep their passed_timestamp + self.params.update({ + u'percent_grade': 20.0, + u'letter_grade': u'', + u'passed': False, + }) + grade = PersistentCourseGrade.update_or_create_course_grade(**self.params) + self.assertEqual(grade.letter_grade, u'') + self.assertEqual(grade.passed_timestamp, passed_timestamp) + + @freeze_time(now()) + def test_passed_timestamp_is_now(self): + grade = PersistentCourseGrade.update_or_create_course_grade(**self.params) + self.assertEqual(now(), grade.passed_timestamp) + def test_create_and_read_grade(self): created_grade = PersistentCourseGrade.update_or_create_course_grade(**self.params) read_grade = PersistentCourseGrade.read_course_grade(self.params["user_id"], self.params["course_id"]) for param in self.params: + if param == u'passed': + continue # passed/passed_timestamp takes special handling, and is tested separately self.assertEqual(self.params[param], getattr(created_grade, param)) + self.assertIsInstance(created_grade.passed_timestamp, datetime) self.assertEqual(created_grade, read_grade) def test_course_version_optional(self): diff --git a/lms/djangoapps/grades/tests/test_new.py b/lms/djangoapps/grades/tests/test_new.py index 0797522769..3f32f87605 100644 --- a/lms/djangoapps/grades/tests/test_new.py +++ b/lms/djangoapps/grades/tests/test_new.py @@ -129,6 +129,13 @@ class TestCourseGradeFactory(GradeTestBase): self.assertEqual(course_grade.letter_grade, u'Pass') self.assertEqual(course_grade.percent, 0.5) + def test_zero_course_grade(self): + grade_factory = CourseGradeFactory(self.request.user) + with mock_get_score(0, 2): + course_grade = grade_factory.create(self.course) + self.assertIsNone(course_grade.letter_grade) + self.assertEqual(course_grade.percent, 0.0) + def test_get_persisted(self): grade_factory = CourseGradeFactory(self.request.user) # first, create a grade in the database