From 726fb0f6f02d9b647f9c48f83690d6c63822eb0b Mon Sep 17 00:00:00 2001 From: jansenk Date: Mon, 28 Jan 2019 10:56:50 -0500 Subject: [PATCH] fail a certificate if grades are modified to failing --- lms/djangoapps/certificates/models.py | 11 ++++ lms/djangoapps/certificates/signals.py | 24 +++++++- .../certificates/tests/test_signals.py | 59 +++++++++++++++++++ lms/djangoapps/courseware/tests/test_views.py | 2 +- .../api/v1/tests/test_gradebook_views.py | 48 ++++++++++++++- lms/djangoapps/grades/course_grade_factory.py | 17 ++++-- .../grades/tests/test_course_grade_factory.py | 2 +- lms/djangoapps/grades/tests/test_tasks.py | 20 +++---- openedx/core/djangoapps/signals/signals.py | 8 +++ 9 files changed, 172 insertions(+), 19 deletions(-) diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index 08ba906556..6a4f108369 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -330,6 +330,17 @@ class GeneratedCertificate(models.Model): self.save() + def mark_notpassing(self, grade): + """ + Invalidates a Generated Certificate by marking it as not passing + """ + self.verify_uuid = '' + self.download_uuid = '' + self.download_url = '' + self.grade = grade + self.status = CertificateStatuses.notpassing + self.save() + def is_valid(self): """ Return True if certificate is valid else return False. diff --git a/lms/djangoapps/certificates/signals.py b/lms/djangoapps/certificates/signals.py index 9bd9219967..4a0bf45aa8 100644 --- a/lms/djangoapps/certificates/signals.py +++ b/lms/djangoapps/certificates/signals.py @@ -9,7 +9,8 @@ from django.dispatch import receiver from lms.djangoapps.certificates.models import ( CertificateGenerationCourseSetting, CertificateWhitelist, - GeneratedCertificate + GeneratedCertificate, + CertificateStatuses ) from lms.djangoapps.certificates.tasks import generate_certificate from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory @@ -17,7 +18,9 @@ from lms.djangoapps.verify_student.services import IDVerificationService from openedx.core.djangoapps.certificates.api import auto_certificate_generation_enabled from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.content.course_overviews.signals import COURSE_PACING_CHANGED -from openedx.core.djangoapps.signals.signals import COURSE_GRADE_NOW_PASSED, LEARNER_NOW_VERIFIED +from openedx.core.djangoapps.signals.signals import (COURSE_GRADE_NOW_PASSED, + LEARNER_NOW_VERIFIED, + COURSE_GRADE_NOW_FAILED) from course_modes.models import CourseMode from student.models import CourseEnrollment @@ -71,6 +74,23 @@ def _listen_for_passing_grade(sender, user, course_id, **kwargs): # pylint: dis )) +@receiver(COURSE_GRADE_NOW_FAILED, dispatch_uid="new_failing_learner") +def _listen_for_failing_grade(sender, user, course_id, grade, **kwargs): # pylint: disable=unused-argument + """ + Listen for a learner failing a course, mark the cert as notpassing + if it is currently passing, + downstream signal from COURSE_GRADE_CHANGED + """ + cert = GeneratedCertificate.certificate_for_student(user, course_id) + if cert is not None: + if CertificateStatuses.is_passing_status(cert.status): + cert.mark_notpassing(grade) + log.info(u'Certificate marked not passing for {user} : {course} via failing grade'.format( + user=user.id, + course=course_id + )) + + @receiver(LEARNER_NOW_VERIFIED, dispatch_uid="learner_track_changed") def _listen_for_id_verification_status_changed(sender, user, **kwargs): # pylint: disable=unused-argument """ diff --git a/lms/djangoapps/certificates/tests/test_signals.py b/lms/djangoapps/certificates/tests/test_signals.py index 4f371acdbd..499a10aa16 100644 --- a/lms/djangoapps/certificates/tests/test_signals.py +++ b/lms/djangoapps/certificates/tests/test_signals.py @@ -222,6 +222,65 @@ class PassingGradeCertsTest(ModuleStoreTestCase): mock_generate_certificate_apply_async.assert_not_called() +@ddt.ddt +class FailingGradeCertsTest(ModuleStoreTestCase): + """ + Tests for marking certificate notpassing when grade goes from passing to failing, + and that the signal has no effect on the cert status if the cert has a non-passing + status + """ + shard = 4 + + def setUp(self): + super(FailingGradeCertsTest, self).setUp() + self.course = CourseFactory.create( + self_paced=True, + ) + self.user = UserFactory.create() + self.enrollment = CourseEnrollmentFactory( + user=self.user, + course_id=self.course.id, + is_active=True, + mode="verified", + ) + attempt = SoftwareSecurePhotoVerification.objects.create( + user=self.user, + status='submitted' + ) + attempt.approve() + + @ddt.data( + CertificateStatuses.deleted, + CertificateStatuses.deleting, + CertificateStatuses.downloadable, + CertificateStatuses.error, + CertificateStatuses.generating, + CertificateStatuses.notpassing, + CertificateStatuses.restricted, + CertificateStatuses.unavailable, + CertificateStatuses.auditing, + CertificateStatuses.audit_passing, + CertificateStatuses.audit_notpassing, + CertificateStatuses.honor_passing, + CertificateStatuses.unverified, + CertificateStatuses.invalidated, + CertificateStatuses.requesting, + ) + def test_cert_failure(self, status): + if CertificateStatuses.is_passing_status(status): + expected_status = CertificateStatuses.notpassing + else: + expected_status = status + GeneratedCertificate.eligible_certificates.create( + user=self.user, + course_id=self.course.id, + status=status + ) + CourseGradeFactory().update(self.user, self.course) + cert = GeneratedCertificate.certificate_for_student(self.user, self.course.id) + self.assertEqual(cert.status, expected_status) + + class LearnerTrackChangeCertsTest(ModuleStoreTestCase): """ Tests for certificate generation task firing on learner verification diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index da7819f33d..f1b0e14f89 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -1456,7 +1456,7 @@ class ProgressPageTests(ProgressPageBaseTests): @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) @ddt.data( - (False, 63, 43), + (False, 64, 44), (True, 55, 39) ) @ddt.unpack diff --git a/lms/djangoapps/grades/api/v1/tests/test_gradebook_views.py b/lms/djangoapps/grades/api/v1/tests/test_gradebook_views.py index cd853aa086..a1e6525830 100644 --- a/lms/djangoapps/grades/api/v1/tests/test_gradebook_views.py +++ b/lms/djangoapps/grades/api/v1/tests/test_gradebook_views.py @@ -29,7 +29,11 @@ from lms.djangoapps.grades.models import ( BlockRecordList, PersistentSubsectionGrade, PersistentSubsectionGradeOverride, - PersistentSubsectionGradeOverrideHistory + PersistentSubsectionGradeOverrideHistory, +) +from lms.djangoapps.certificates.models import ( + GeneratedCertificate, + CertificateStatuses, ) from lms.djangoapps.grades.subsection_grade import ReadSubsectionGrade from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory @@ -1148,6 +1152,48 @@ class GradebookBulkUpdateViewTest(GradebookViewTestBase): self.assertEqual(audit_item.feature, PersistentSubsectionGradeOverrideHistory.GRADEBOOK) self.assertEqual(audit_item.action, PersistentSubsectionGradeOverrideHistory.CREATE_OR_UPDATE) + def test_update_failing_grade(self): + """ + Test that when we update a user's grade to failing, their certificate is marked notpassing + """ + with override_waffle_flag(self.waffle_flag, active=True): + GeneratedCertificate.eligible_certificates.create( + user=self.student, + course_id=self.course.id, + status=CertificateStatuses.downloadable, + ) + self.login_staff() + post_data = [ + { + 'user_id': self.student.id, + 'usage_id': text_type(self.subsections[self.chapter_1.location][0].location), + 'grade': { + 'earned_all_override': 0, + 'possible_all_override': 3, + 'earned_graded_override': 0, + 'possible_graded_override': 2, + }, + }, + { + 'user_id': self.student.id, + 'usage_id': text_type(self.subsections[self.chapter_1.location][1].location), + 'grade': { + 'earned_all_override': 0, + 'possible_all_override': 4, + 'earned_graded_override': 0, + 'possible_graded_override': 4, + }, + } + ] + resp = self.client.post( + self.get_url(), + data=json.dumps(post_data), + content_type='application/json', + ) + self.assertEqual(status.HTTP_202_ACCEPTED, resp.status_code) + cert = GeneratedCertificate.certificate_for_student(self.student, self.course.id) + self.assertEqual(cert.status, CertificateStatuses.notpassing) + @ddt.ddt class SubsectionGradeViewTest(GradebookViewTestBase): diff --git a/lms/djangoapps/grades/course_grade_factory.py b/lms/djangoapps/grades/course_grade_factory.py index e29a0fed8c..13de50ce6b 100644 --- a/lms/djangoapps/grades/course_grade_factory.py +++ b/lms/djangoapps/grades/course_grade_factory.py @@ -6,7 +6,9 @@ from logging import getLogger from six import text_type -from openedx.core.djangoapps.signals.signals import COURSE_GRADE_CHANGED, COURSE_GRADE_NOW_PASSED +from openedx.core.djangoapps.signals.signals import (COURSE_GRADE_CHANGED, + COURSE_GRADE_NOW_PASSED, + COURSE_GRADE_NOW_FAILED) from .config import assume_zero_if_absent, should_persist_grades from .course_data import CourseData @@ -162,11 +164,11 @@ class CourseGradeFactory(object): """ Computes, saves, and returns a CourseGrade object for the given user and course. - Sends a COURSE_GRADE_CHANGED signal to listeners and a - COURSE_GRADE_NOW_PASSED if learner has passed course. + Sends a COURSE_GRADE_CHANGED signal to listeners and + COURSE_GRADE_NOW_PASSED if learner has passed course or + COURSE_GRADE_NOW_FAILED if learner is now failing course """ should_persist = should_persist_grades(course_data.course_key) - if should_persist and force_update_subsections: prefetch(user, course_data.course_key) @@ -204,6 +206,13 @@ class CourseGradeFactory(object): user=user, course_id=course_data.course_key, ) + else: + COURSE_GRADE_NOW_FAILED.send( + sender=CourseGradeFactory, + user=user, + course_id=course_data.course_key, + grade=course_grade, + ) log.info( u'Grades: Update, %s, User: %s, %s, persisted: %s', diff --git a/lms/djangoapps/grades/tests/test_course_grade_factory.py b/lms/djangoapps/grades/tests/test_course_grade_factory.py index 4b66ba447b..aea2cfd4d8 100644 --- a/lms/djangoapps/grades/tests/test_course_grade_factory.py +++ b/lms/djangoapps/grades/tests/test_course_grade_factory.py @@ -119,7 +119,7 @@ class TestCourseGradeFactory(GradeTestBase): with self.assertNumQueries(5): _assert_read(expected_pass=True, expected_percent=1.0) # updated to grade of 1.0 - num_queries = 29 + num_queries = 30 with self.assertNumQueries(num_queries), mock_get_score(0, 0): # the subsection now is worth zero grade_factory.update(self.request.user, self.course, force_update_subsections=True) diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 1b3306e5db..488d0f3eaa 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -176,10 +176,10 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEquals(mock_block_structure_create.call_count, 1) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 35, True), - (ModuleStoreEnum.Type.mongo, 1, 35, False), - (ModuleStoreEnum.Type.split, 3, 35, True), - (ModuleStoreEnum.Type.split, 3, 35, False), + (ModuleStoreEnum.Type.mongo, 1, 36, True), + (ModuleStoreEnum.Type.mongo, 1, 36, False), + (ModuleStoreEnum.Type.split, 3, 36, True), + (ModuleStoreEnum.Type.split, 3, 36, False), ) @ddt.unpack def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections): @@ -191,8 +191,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self._apply_recalculate_subsection_grade() @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 35), - (ModuleStoreEnum.Type.split, 3, 35), + (ModuleStoreEnum.Type.mongo, 1, 36), + (ModuleStoreEnum.Type.split, 3, 36), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): @@ -237,8 +237,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest ) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 18), - (ModuleStoreEnum.Type.split, 3, 18), + (ModuleStoreEnum.Type.mongo, 1, 19), + (ModuleStoreEnum.Type.split, 3, 19), ) @ddt.unpack def test_persistent_grades_not_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): @@ -252,8 +252,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEqual(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 36), - (ModuleStoreEnum.Type.split, 3, 36), + (ModuleStoreEnum.Type.mongo, 1, 37), + (ModuleStoreEnum.Type.split, 3, 37), ) @ddt.unpack def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): diff --git a/openedx/core/djangoapps/signals/signals.py b/openedx/core/djangoapps/signals/signals.py index 90b9fa08a6..d76aa1ea3a 100644 --- a/openedx/core/djangoapps/signals/signals.py +++ b/openedx/core/djangoapps/signals/signals.py @@ -20,6 +20,14 @@ COURSE_GRADE_NOW_PASSED = Signal( 'course_id', # course.id ] ) +#Signal that indicates a user is now failing a course that they had previously passed. +COURSE_GRADE_NOW_FAILED = Signal( + providing_args=[ + 'user', # user object + 'course_id', # course.id + 'grade', # CourseGrade object + ] +) # Signal that indicates that a user has become verified LEARNER_NOW_VERIFIED = Signal(providing_args=['user'])