Merge pull request #14736 from edx/noraiz/ECOM-7522

ECOM-7522 Fix wrongly failed requirment to earn credit

openedx/core/djangoapps/credit/models.py

@@ -465,8 +465,15 @@ class CreditRequirementStatus(TimeStampedModel):
             defaults={"reason": reason, "status": status}
         )
         if not created:
+            # do not update status to `failed` if user has `satisfied` the requirement
+            if status == 'failed' and requirement_status.status == 'satisfied':
+                log.info(
+                    u'Can not change status of credit requirement "%s" from satisfied to failed ',
+                    requirement_status.requirement_id
+                )
+                return
             requirement_status.status = status
-            requirement_status.reason = reason if reason else {}
+            requirement_status.reason = reason
             requirement_status.save()
 
     @classmethod

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

@@ -435,18 +435,14 @@ class CreditRequirementApiTests(CreditApiTestBase):
         self.assertEqual(req_status[0]["order"], expected_order)
         return req_status
 
-    @ddt.data(
-        *CourseMode.CREDIT_ELIGIBLE_MODES
-    )
-    def test_set_credit_requirement_status(self, mode):
-        username = self.user.username
-        credit_course = self.add_credit_course()
+    def _set_credit_course_requirements(self):
 
-        # Enroll user and verify his enrollment.
-        self.enroll(self.user, self.course_key, mode)
-        self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course_key))
-        self.assertTrue(CourseEnrollment.enrollment_mode_for_user(self.user, self.course_key), (mode, True))
+        """
+        Sets requirements for the credit course.
 
+        Returns:
+            dict: Course requirements
+        """
         requirements = [
             {
                 "namespace": "grade",
@@ -467,6 +463,23 @@ class CreditRequirementApiTests(CreditApiTestBase):
         course_requirements = api.get_credit_requirements(self.course_key)
         self.assertEqual(len(course_requirements), 2)
 
+    @ddt.data(
+        *CourseMode.CREDIT_ELIGIBLE_MODES
+    )
+    def test_set_credit_requirement_status(self, mode):
+        """
+        Test set/update credit requirement status
+        """
+        username = self.user.username
+        credit_course = self.add_credit_course()
+
+        # Enroll user and verify his enrollment.
+        self.enroll(self.user, self.course_key, mode)
+        self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course_key))
+        self.assertTrue(CourseEnrollment.enrollment_mode_for_user(self.user, self.course_key), (mode, True))
+
+        self._set_credit_course_requirements()
+
         # Initially, the status should be None
         self.assert_grade_requirement_status(None, 0)
 
@@ -480,16 +493,20 @@ class CreditRequirementApiTests(CreditApiTestBase):
         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(self.user, self.course_key, "grade", "grade")
-        self.assert_grade_requirement_status('satisfied', 0)
+        # order of below two assertions matter as:
+        # `failed` to `satisfied` is allowed
+        # `satisfied` to `failed` is not allowed
 
-        # Set the requirement to "failed" and check that it's actually set
+        # 1. Set the requirement to "failed" and check that it's actually set
         api.set_credit_requirement_status(self.user, self.course_key, "grade", "grade", status="failed")
         self.assert_grade_requirement_status('failed', 0)
 
+        # 2. Set the requirement to "satisfied" and check that it's actually set
+        api.set_credit_requirement_status(self.user, self.course_key, "grade", "grade")
+        self.assert_grade_requirement_status('satisfied', 0)
+
         req_status = api.get_credit_requirement_status(self.course_key, username)
-        self.assertEqual(req_status[0]["status"], "failed")
+        self.assertEqual(req_status[0]["status"], "satisfied")
         self.assertEqual(req_status[0]["order"], 0)
 
         # make sure the 'order' on the 2nd requirement is set correctly (aka 1)
@@ -511,6 +528,32 @@ class CreditRequirementApiTests(CreditApiTestBase):
         )
         self.assertEqual(req_status[0]["status"], "declined")
 
+    @ddt.data(
+        *CourseMode.CREDIT_ELIGIBLE_MODES
+    )
+    def test_set_credit_requirement_status_satisfied_to_failed(self, mode):
+        """
+        Test that if credit requirment status is set to `satisfied`, it
+        can not not be changed to `failed`
+        """
+        self.add_credit_course()
+
+        # Enroll user and verify enrollment.
+        self.enroll(self.user, self.course_key, mode)
+        self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course_key))
+        self.assertTrue(CourseEnrollment.enrollment_mode_for_user(self.user, self.course_key), (mode, True))
+
+        self._set_credit_course_requirements()
+
+        api.set_credit_requirement_status(self.user, self.course_key, "grade", "grade", status="satisfied")
+        self.assert_grade_requirement_status('satisfied', 0)
+
+        # try to set status to `failed`
+        api.set_credit_requirement_status(self.user, self.course_key, "grade", "grade", status="failed")
+
+        # status should not be changed to `failed`, rather should maintain already set status `satisfied`
+        self.assert_grade_requirement_status('satisfied', 0)
+
     @ddt.data(
         *CourseMode.CREDIT_ELIGIBLE_MODES
     )