Updated credit requirements logic

Credit eligibility can be updated up to the point that the course is closed or the student submits a request for credit. Credit eligibility cannot be removed.

ECOM-4379

openedx/core/djangoapps/credit/api/eligibility.py

@@ -7,13 +7,10 @@ import logging
 
 from opaque_keys.edx.keys import CourseKey
 
-from openedx.core.djangoapps.credit.exceptions import InvalidCreditRequirements, InvalidCreditCourse
 from openedx.core.djangoapps.credit.email_utils import send_credit_notifications
+from openedx.core.djangoapps.credit.exceptions import InvalidCreditRequirements, InvalidCreditCourse
 from openedx.core.djangoapps.credit.models import (
-    CreditCourse,
-    CreditRequirement,
-    CreditRequirementStatus,
-    CreditEligibility,
+    CreditCourse, CreditRequirement, CreditRequirementStatus, CreditEligibility, CreditRequest
 )
 
 # TODO: Cleanup this mess! ECOM-2908
@@ -228,13 +225,21 @@ def set_credit_requirement_status(username, course_key, req_namespace, req_name,
             )
 
     """
-    # Check if we're already eligible for credit.
-    # If so, short-circuit this process.
-    if CreditEligibility.is_user_eligible_for_credit(course_key, username):
+    # Do not allow students who have requested credit to change their eligibility
+    if CreditRequest.get_user_request_status(username, course_key):
+        log.info(
+            u'Refusing to set status of requirement with namespace "%s" and name "%s" because the '
+            u'user "%s" has already requested credit for the course "%s".',
+            req_namespace, req_name, username, course_key
+        )
+        return
+
+    # Do not allow a student who has earned eligibility to un-earn eligibility
+    eligible_before_update = CreditEligibility.is_user_eligible_for_credit(course_key, username)
+    if eligible_before_update and status == 'failed':
         log.info(
-            u'Skipping update of credit requirement with namespace "%s" '
-            u'and name "%s" because the user "%s" is already eligible for credit '
-            u'in the course "%s".',
+            u'Refusing to set status of requirement with namespace "%s" and name "%s" to "failed" because the '
+            u'user "%s" is already eligible for credit in the course "%s".',
             req_namespace, req_name, username, course_key
         )
         return
@@ -273,9 +278,9 @@ def set_credit_requirement_status(username, course_key, req_namespace, req_name,
         username, req_to_update, status=status, reason=reason
     )
 
-    # If we're marking this requirement as "satisfied", there's a chance
-    # that the user has met all eligibility requirements.
-    if status == "satisfied":
+    # If we're marking this requirement as "satisfied", there's a chance that the user has met all eligibility
+    # requirements, and should be notified. However, if the user was already eligible, do not send another notification.
+    if status == "satisfied" and not eligible_before_update:
         is_eligible, eligibility_record_created = CreditEligibility.update_eligibility(reqs, username, course_key)
         if eligibility_record_created and is_eligible:
             try:

openedx/core/djangoapps/credit/signals.py

@@ -7,11 +7,10 @@ import logging
 from django.dispatch import receiver
 from django.utils import timezone
 from opaque_keys.edx.keys import CourseKey
-
-from openedx.core.djangoapps.signals.signals import GRADES_UPDATED
-from openedx.core.djangoapps.credit.verification_access import update_verification_partitions
 from xmodule.modulestore.django import SignalHandler
 
+from openedx.core.djangoapps.credit.verification_access import update_verification_partitions
+from openedx.core.djangoapps.signals.signals import GRADES_UPDATED
 
 log = logging.getLogger(__name__)
 
@@ -80,12 +79,39 @@ def listen_for_grade_calculation(sender, username, grade_summary, course_key, de
             criteria = requirements[0].get('criteria')
             if criteria:
                 min_grade = criteria.get('min_grade')
-                if grade_summary['percent'] >= min_grade:
-                    reason_dict = {'final_grade': grade_summary['percent']}
-                    api.set_credit_requirement_status(
-                        username, course_id, 'grade', 'grade', status="satisfied", reason=reason_dict
-                    )
-                elif deadline and deadline < timezone.now():
+                passing_grade = grade_summary['percent'] >= min_grade
+                now = timezone.now()
+                status = None
+                reason = None
+
+                if (deadline and now < deadline) or not deadline:
+                    # Student completed coursework on-time
+
+                    if passing_grade:
+                        # Student received a passing grade
+                        status = 'satisfied'
+                        reason = {'final_grade': grade_summary['percent']}
+                else:
+                    # Submission after deadline
+
+                    if passing_grade:
+                        # Grade was good, but submission arrived too late
+                        status = 'failed'
+                        reason = {
+                            'current_date': now,
+                            'deadline': deadline
+                        }
+                    else:
+                        # Student failed to receive minimum grade
+                        status = 'failed'
+                        reason = {
+                            'final_grade': grade_summary['percent'],
+                            'minimum_grade': min_grade
+                        }
+
+                # We do not record a status if the user has not yet earned the minimum grade, but still has
+                # time to do so.
+                if status and reason:
                     api.set_credit_requirement_status(
-                        username, course_id, 'grade', 'grade', status="failed", reason={}
+                        username, course_id, 'grade', 'grade', status=status, reason=reason
                     )

openedx/core/djangoapps/credit/tests/test_api.py

@@ -33,7 +33,8 @@ from openedx.core.djangoapps.credit.models import (
     CreditProvider,
     CreditRequirement,
     CreditRequirementStatus,
-    CreditEligibility
+    CreditEligibility,
+    CreditRequest
 )
 from student.tests.factories import UserFactory
 from util.date_utils import from_timestamp
@@ -380,7 +381,7 @@ class CreditRequirementApiTests(CreditApiTestBase):
 
     def test_set_credit_requirement_status(self):
         username = "staff"
-        self.add_credit_course()
+        credit_course = self.add_credit_course()
         requirements = [
             {
                 "namespace": "grade",
@@ -405,6 +406,16 @@ class CreditRequirementApiTests(CreditApiTestBase):
         # Initially, the status should be None
         self.assert_grade_requirement_status(None, 0)
 
+        # Requirement statuses cannot be changed if a CreditRequest exists
+        credit_request = CreditRequest.objects.create(
+            course=credit_course,
+            provider=CreditProvider.objects.first(),
+            username=username,
+        )
+        api.set_credit_requirement_status(username, self.course_key, "grade", "grade")
+        self.assert_grade_requirement_status(None, 0)
+        credit_request.delete()
+
         # Set the requirement to "satisfied" and check that it's actually set
         api.set_credit_requirement_status(username, self.course_key, "grade", "grade")
         self.assert_grade_requirement_status('satisfied', 0)
@@ -532,7 +543,7 @@ class CreditRequirementApiTests(CreditApiTestBase):
         user = UserFactory.create(username=self.USER_INFO['username'], password=self.USER_INFO['password'])
 
         # Satisfy one of the requirements, but not the other
-        with self.assertNumQueries(11):
+        with self.assertNumQueries(12):
             api.set_credit_requirement_status(
                 user.username,
                 self.course_key,
@@ -544,7 +555,7 @@ class CreditRequirementApiTests(CreditApiTestBase):
         self.assertFalse(api.is_user_eligible_for_credit("bob", self.course_key))
 
         # Satisfy the other requirement
-        with self.assertNumQueries(19):
+        with self.assertNumQueries(20):
             api.set_credit_requirement_status(
                 "bob",
                 self.course_key,
@@ -598,7 +609,7 @@ class CreditRequirementApiTests(CreditApiTestBase):
         # Delete the eligibility entries and satisfy the user's eligibility
         # requirement again to trigger eligibility notification
         CreditEligibility.objects.all().delete()
-        with self.assertNumQueries(15):
+        with self.assertNumQueries(16):
             api.set_credit_requirement_status(
                 "bob",
                 self.course_key,

openedx/core/djangoapps/credit/tests/test_signals.py

@@ -70,20 +70,32 @@ class TestMinGradedRequirementStatus(ModuleStoreTestCase):
         """ Verify the user's credit requirement status is as expected after simulating a grading calculation. """
         listen_for_grade_calculation(None, self.user.username, {'percent': grade}, self.course.id, due_date)
         req_status = get_credit_requirement_status(self.course.id, self.request.user.username, 'grade', 'grade')
+
         self.assertEqual(req_status[0]['status'], expected_status)
 
+        if expected_status == 'satisfied':
+            expected_reason = {'final_grade': grade}
+            self.assertEqual(req_status[0]['reason'], expected_reason)
+
     @ddt.data(
         (0.6, VALID_DUE_DATE),
-        (0.52, VALID_DUE_DATE),
-        (0.70, EXPIRED_DUE_DATE),
+        (0.52, None),
     )
     @ddt.unpack
     def test_min_grade_requirement_with_valid_grade(self, grade, due_date):
-        """Test with valid grades. Deadline date does not effect in case
-        of valid grade.
-        """
+        """Test with valid grades submitted before deadline"""
         self.assert_requirement_status(grade, due_date, 'satisfied')
 
+    def test_grade_changed(self):
+        """ Verify successive calls to update a satisfied grade requirement are recorded. """
+        self.assert_requirement_status(0.6, self.VALID_DUE_DATE, 'satisfied')
+        self.assert_requirement_status(0.75, self.VALID_DUE_DATE, 'satisfied')
+        self.assert_requirement_status(0.70, self.VALID_DUE_DATE, 'satisfied')
+
+    def test_min_grade_requirement_with_valid_grade_and_expired_deadline(self):
+        """ Verify the status is set to failure if a passing grade is received past the submission deadline. """
+        self.assert_requirement_status(0.70, self.EXPIRED_DUE_DATE, 'failed')
+
     @ddt.data(
         (0.50, None),
         (0.51, None),