Merge pull request #9386 from edx/cdodge/dont-return-none-for-non-credit-course

Cdodge/dont return none for non credit course

openedx/core/djangoapps/credit/services.py

@@ -31,6 +31,22 @@ class CreditService(object):
     Course Credit XBlock service
     """
 
+    def is_credit_course(self, course_key_or_id):
+        """
+        Returns boolean if the passed in course_id (string) or course_key is
+        a credit_course
+        """
+
+        # 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,
+        )
+
+        course_key = _get_course_key(course_key_or_id)
+
+        return is_credit_course(course_key)
+
     def get_credit_state(self, user_id, course_key_or_id):
         """
         Return all information about the user's credit state inside of a given
@@ -46,6 +62,7 @@ class CreditService(object):
             {
                 'enrollment_mode': the mode that the user is enrolled in the course
                 'profile_fullname': the name that the student registered under, used for verification
+                'is_credit_course': if the course has been marked as a credit bearing course
                 'credit_requirement_status': the user's status in fulfilling those requirements
             }
         """
@@ -72,12 +89,10 @@ class CreditService(object):
             # not enrolled
             return None
 
-        if not is_credit_course(course_key):
-            return None
-
         return {
             'enrollment_mode': enrollment.mode,
             'profile_fullname': user.profile.name,
+            'is_credit_course': is_credit_course(course_key),
             'credit_requirement_status': get_credit_requirement_status(course_key, user.username)
         }
 
@@ -90,6 +105,19 @@ class CreditService(object):
         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,
+            set_credit_requirement_status as api_set_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 update activity to the credit requirements
         # table. This will be to help debug any issues that might
         # arise in production
@@ -114,14 +142,6 @@ class CreditService(object):
         except ObjectDoesNotExist:
             return None
 
-        course_key = _get_course_key(course_key_or_id)
-
-        # 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 (
-            set_credit_requirement_status as api_set_credit_requirement_status
-        )
-
         api_set_credit_requirement_status(
             user.username,
             course_key,

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

@@ -63,7 +63,9 @@ class CreditServiceTests(ModuleStoreTestCase):
         self.credit_course.enabled = False
         self.credit_course.save()
 
-        self.assertIsNone(self.service.get_credit_state(self.user.id, self.course.id))
+        credit_state = self.service.get_credit_state(self.user.id, self.course.id)
+        self.assertIsNotNone(credit_state)
+        self.assertFalse(credit_state['is_credit_course'])
 
     def test_no_profile_name(self):
         """
@@ -82,6 +84,8 @@ class CreditServiceTests(ModuleStoreTestCase):
         Happy path through the service
         """
 
+        self.assertTrue(self.service.is_credit_course(self.course.id))
+
         CourseEnrollment.enroll(self.user, self.course.id)
 
         # set course requirements
@@ -110,12 +114,39 @@ class CreditServiceTests(ModuleStoreTestCase):
         credit_state = self.service.get_credit_state(self.user.id, self.course.id)
 
         self.assertIsNotNone(credit_state)
+        self.assertTrue(credit_state['is_credit_course'])
         self.assertEqual(credit_state['enrollment_mode'], 'honor')
         self.assertEqual(credit_state['profile_fullname'], 'Foo Bar')
         self.assertEqual(len(credit_state['credit_requirement_status']), 1)
         self.assertEqual(credit_state['credit_requirement_status'][0]['name'], 'grade')
         self.assertEqual(credit_state['credit_requirement_status'][0]['status'], 'satisfied')
 
+    def test_set_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.set_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_bad_user(self):
         """
         Try setting requirements status with a bad user_id