Merge pull request #9536 from…

Merge pull request #9536 from edx/muhhshoaib/PHX-122-remove-the-record-in-the-creditRequirementStatus-table

PHX-122

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

@@ -284,6 +284,59 @@ def set_credit_requirement_status(username, course_key, req_namespace, req_name,
                 log.error("Error sending email")
 
 
+# pylint: disable=invalid-name
+def remove_credit_requirement_status(username, course_key, req_namespace, req_name):
+    """
+    Remove the user's requirement status.
+
+    This will remove the record from the credit requirement status table.
+    The user will still be eligible for the credit in a course.
+
+    Args:
+        username (str): Username of the user
+        course_key (CourseKey): Identifier for the course associated
+                                with the requirement.
+        req_namespace (str): Namespace of the requirement
+                            (e.g. "grade" or "reverification")
+        req_name (str): Name of the requirement
+                        (e.g. "grade" or the location of the ICRV XBlock)
+
+    Example:
+        >>> remove_credit_requirement_status(
+                "staff",
+                CourseKey.from_string("course-v1-edX-DemoX-1T2015"),
+                "reverification",
+                "i4x://edX/DemoX/edx-reverification-block/assessment_uuid".
+            )
+
+    """
+
+    # Find the requirement we're trying to remove
+    req_to_remove = CreditRequirement.get_course_requirements(course_key, namespace=req_namespace, name=req_name)
+
+    # If we can't find the requirement, then the most likely explanation
+    # is that there was a lag removing the credit requirements after the course
+    # was published.  We *could* attempt to remove the requirement here,
+    # but that could cause serious performance issues if many users attempt to
+    # lock the row at the same time.
+    # Instead, we skip removing the requirement and log an error.
+    if req_to_remove is None:
+        log.error(
+            (
+                u'Could not remove credit requirement in course "%s" '
+                u'with namespace "%s" and name "%s" '
+                u'because the requirement does not exist. '
+            ),
+            unicode(course_key), req_namespace, req_name
+        )
+        return
+
+    # Remove the requirement status
+    CreditRequirementStatus.remove_requirement_status(
+        username, req_to_remove
+    )
+
+
 def get_credit_requirement_status(course_key, username, namespace=None, name=None):
     """ Retrieve the user's status for each credit requirement in the course.
 

openedx/core/djangoapps/credit/models.py

@@ -467,6 +467,24 @@ class CreditRequirementStatus(TimeStampedModel):
             requirement_status.reason = reason if reason else {}
             requirement_status.save()
 
+    @classmethod
+    @transaction.commit_on_success
+    def remove_requirement_status(cls, username, requirement):
+        """
+        Remove credit requirement status for given username.
+
+        Args:
+            username(str): Username of the user
+            requirement(CreditRequirement): 'CreditRequirement' object
+        """
+
+        try:
+            requirement_status = cls.objects.get(username=username, requirement=requirement)
+            requirement_status.delete()
+        except cls.DoesNotExist:
+            log.exception(u'The requirement status does not exist against the username %s.', username)
+            return
+
 
 class CreditEligibility(TimeStampedModel):
     """

openedx/core/djangoapps/credit/services.py

@@ -159,3 +159,54 @@ class CreditService(object):
             status,
             reason
         )
+
+    def remove_credit_requirement_status(self, user_id, course_key_or_id, req_namespace, req_name):
+        """
+        A simple wrapper around the method of the same name in
+        api.eligibility.py. The only difference is that a user_id
+        is passed in.
+
+        For more information, see documentation on this method name
+        in api.eligibility.py
+        """
+
+        # This seems to need to be here otherwise we get
+        # circular references when starting up the app
+        from openedx.core.djangoapps.credit.api.eligibility import (
+            is_credit_course,
+            remove_credit_requirement_status as api_remove_credit_requirement_status
+        )
+
+        course_key = _get_course_key(course_key_or_id)
+
+        # quick exit, if course is not credit enabled
+        if not is_credit_course(course_key):
+            return
+
+        # always log any deleted activity to the credit requirements
+        # table. This will be to help debug any issues that might
+        # arise in production
+        log_msg = (
+            'remove_credit_requirement_status was called with '
+            'user_id={user_id}, course_key_or_id={course_key_or_id} '
+            'req_namespace={req_namespace}, req_name={req_name}, '.format(
+                user_id=user_id,
+                course_key_or_id=course_key_or_id,
+                req_namespace=req_namespace,
+                req_name=req_name
+            )
+        )
+        log.info(log_msg)
+
+        # need to get user_name from the user object
+        try:
+            user = User.objects.get(id=user_id)
+        except ObjectDoesNotExist:
+            return None
+
+        api_remove_credit_requirement_status(
+            user.username,
+            course_key,
+            req_namespace,
+            req_name
+        )

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

@@ -318,6 +318,63 @@ class CreditRequirementApiTests(CreditApiTestBase):
         req_status = api.get_credit_requirement_status(self.course_key, "staff", namespace="grade", name="grade")
         self.assertEqual(req_status[0]["status"], "failed")
 
+    def test_remove_credit_requirement_status(self):
+        self.add_credit_course()
+        requirements = [
+            {
+                "namespace": "grade",
+                "name": "grade",
+                "display_name": "Grade",
+                "criteria": {
+                    "min_grade": 0.8
+                },
+            },
+            {
+                "namespace": "reverification",
+                "name": "i4x://edX/DemoX/edx-reverification-block/assessment_uuid",
+                "display_name": "Assessment 1",
+                "criteria": {},
+            }
+        ]
+
+        api.set_credit_requirements(self.course_key, requirements)
+        course_requirements = api.get_credit_requirements(self.course_key)
+        self.assertEqual(len(course_requirements), 2)
+
+        # before setting credit_requirement_status
+        api.remove_credit_requirement_status("staff", self.course_key, "grade", "grade")
+        req_status = api.get_credit_requirement_status(self.course_key, "staff", namespace="grade", name="grade")
+        self.assertIsNone(req_status[0]["status"])
+        self.assertIsNone(req_status[0]["status_date"])
+        self.assertIsNone(req_status[0]["reason"])
+
+        # Set the requirement to "satisfied" and check that it's actually set
+        api.set_credit_requirement_status("staff", self.course_key, "grade", "grade")
+        req_status = api.get_credit_requirement_status(self.course_key, "staff", namespace="grade", name="grade")
+        self.assertEqual(len(req_status), 1)
+        self.assertEqual(req_status[0]["status"], "satisfied")
+
+        # remove the credit requirement status and check that it's actually removed
+        api.remove_credit_requirement_status("staff", self.course_key, "grade", "grade")
+        req_status = api.get_credit_requirement_status(self.course_key, "staff", namespace="grade", name="grade")
+        self.assertIsNone(req_status[0]["status"])
+        self.assertIsNone(req_status[0]["status_date"])
+        self.assertIsNone(req_status[0]["reason"])
+
+    def test_remove_credit_requirement_status_req_not_configured(self):
+        # Configure a credit course with no requirements
+        self.add_credit_course()
+
+        # A user satisfies a requirement. This could potentially
+        # happen if there's a lag when the requirements are removed
+        # after the course is published.
+        api.remove_credit_requirement_status("bob", self.course_key, "grade", "grade")
+
+        # Since the requirement hasn't been published yet, it won't show
+        # up in the list of requirements.
+        req_status = api.get_credit_requirement_status(self.course_key, "bob", namespace="grade", name="grade")
+        self.assertEqual(len(req_status), 0)
+
     def test_satisfy_all_requirements(self):
         """ Test the credit requirements, eligibility notification, email
         content caching for a credit course.

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

@@ -121,6 +121,117 @@ class CreditServiceTests(ModuleStoreTestCase):
         self.assertEqual(credit_state['credit_requirement_status'][0]['name'], 'grade')
         self.assertEqual(credit_state['credit_requirement_status'][0]['status'], 'satisfied')
 
+    def test_remove_credit_requirement_status(self):
+        """
+        Happy path when deleting the requirement status.
+        """
+        self.assertTrue(self.service.is_credit_course(self.course.id))
+
+        CourseEnrollment.enroll(self.user, self.course.id)
+
+        # set course requirements
+        set_credit_requirements(
+            self.course.id,
+            [
+                {
+                    "namespace": "grade",
+                    "name": "grade",
+                    "display_name": "Grade",
+                    "criteria": {
+                        "min_grade": 0.8
+                    },
+                },
+            ]
+        )
+
+        # mark the grade as satisfied
+        self.service.set_credit_requirement_status(
+            self.user.id,
+            self.course.id,
+            'grade',
+            'grade'
+        )
+
+        # now the status should be "satisfied" when looking at the credit_requirement_status list
+        credit_state = self.service.get_credit_state(self.user.id, self.course.id)
+        self.assertEqual(credit_state['credit_requirement_status'][0]['status'], "satisfied")
+
+        # remove the requirement status.
+        self.service.remove_credit_requirement_status(
+            self.user.id,
+            self.course.id,
+            'grade',
+            'grade'
+        )
+
+        # now the status should be None when looking at the credit_requirement_status list
+        credit_state = self.service.get_credit_state(self.user.id, self.course.id)
+        self.assertEqual(credit_state['credit_requirement_status'][0]['status'], None)
+
+    def test_invalid_user(self):
+        """
+        Try removing requirement status with a invalid user_id
+        """
+
+        # set course requirements
+        set_credit_requirements(
+            self.course.id,
+            [
+                {
+                    "namespace": "grade",
+                    "name": "grade",
+                    "display_name": "Grade",
+                    "criteria": {
+                        "min_grade": 0.8
+                    },
+                },
+            ]
+        )
+
+        # mark the grade as satisfied
+        retval = self.service.set_credit_requirement_status(
+            self.user.id,
+            self.course.id,
+            'grade',
+            'grade'
+        )
+        self.assertIsNone(retval)
+
+        # remove the requirement status with the invalid user id
+        retval = self.service.remove_credit_requirement_status(
+            0,
+            self.course.id,
+            'grade',
+            'grade'
+        )
+        self.assertIsNone(retval)
+
+    def test_remove_status_non_credit(self):
+        """
+        assert that we can still try to update a credit status but return quickly if
+        a course is not credit eligible
+        """
+
+        no_credit_course = CourseFactory.create(org='NoCredit', number='NoCredit', display_name='Demo_Course')
+
+        self.assertFalse(self.service.is_credit_course(no_credit_course.id))
+
+        CourseEnrollment.enroll(self.user, no_credit_course.id)
+
+        # this should be a no-op
+        self.service.remove_credit_requirement_status(
+            self.user.id,
+            no_credit_course.id,
+            'grade',
+            'grade'
+        )
+
+        credit_state = self.service.get_credit_state(self.user.id, no_credit_course.id)
+
+        self.assertIsNotNone(credit_state)
+        self.assertFalse(credit_state['is_credit_course'])
+        self.assertEqual(len(credit_state['credit_requirement_status']), 0)
+
     def test_course_name(self):
         """
         Make sure we can get back the optional course name