Merge pull request #29834 from mitodl/arslan/286-catch-courseoverview-ex

fix: catch and log the CourseOverview.DoesNotExist instead of raising

Author: pdpinch

openedx/core/djangoapps/credit/services.py

@@ -1,6 +1,7 @@
 """
 Implementation of "credit" XBlock service
 """
+from datetime import datetime, timedelta
 import logging
 
 from django.contrib.auth.models import User  # lint-amnesty, pylint: disable=imported-auth-user
@@ -100,11 +101,30 @@ def get_credit_state(self, user_id, course_key_or_id, return_course_info=False):
         }
 
         if return_course_info:
-            course_overview = CourseOverview.get_from_id(course_key)
-            result.update({
-                'course_name': course_overview.display_name,
-                'course_end_date': course_overview.end,
-            })
+            try:
+                course_overview = CourseOverview.get_from_id(course_key)
+                result.update({
+                    'course_name': course_overview.display_name,
+                    'course_end_date': course_overview.end,
+                })
+            except CourseOverview.DoesNotExist:
+                # NOTE: Since the caller requested "return_course_info=True" and we don't have course to get that info.
+                # Also, The "get_credit_state" is called from several places directly or indirectly (Mostly from
+                # exams/grades services) which relatively depend upon course end date.
+                # As per the current structure of edX the exam attempts can exist without a course and they can request
+                # credit state so, the safest options would be to send some date/time in the past (One hour in past to
+                # be safe) so that those attempts can be marked or treated as expired/completed instead of any other
+                # status that could be error prone.
+
+                one_hour_past = datetime.now() - timedelta(hours=1)
+                result.update({
+                    'course_end_date': one_hour_past,
+                })
+                log.exception(
+                    "Could not get name and end_date for course %s, This happened because we were unable to "
+                    "get/create CourseOverview object for the course. It's possible that the Course has been deleted.",
+                    str(course_key),
+                )
         return result
 
     def set_credit_requirement_status(self, user_id, course_key_or_id, req_namespace,

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

@@ -3,9 +3,12 @@
 """
 
 
+from unittest.mock import patch
+from datetime import datetime
 import ddt
 
 from common.djangoapps.course_modes.models import CourseMode
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from openedx.core.djangoapps.credit.api.eligibility import set_credit_requirements
 from openedx.core.djangoapps.credit.models import CreditCourse
 from openedx.core.djangoapps.credit.services import CreditService
@@ -261,6 +264,26 @@ def test_course_name(self):
         assert 'course_name' in credit_state
         assert credit_state['course_name'] == self.course.display_name
 
+    @patch("openedx.core.djangoapps.credit.services.log")
+    def test_get_info_from_non_existent_course(self, exception_log):
+        """
+        Make sure we catch the CourseOverview.DoesNotExist exception and log it instead of raising
+        """
+        with patch("openedx.core.djangoapps.content.course_overviews.models.CourseOverview.get_from_id",
+                   side_effect=CourseOverview.DoesNotExist):
+
+            self.enroll()
+            credit_state = self.service.get_credit_state(self.user.id, self.course.id, return_course_info=True)
+
+            assert credit_state is not None
+            # When exception is caught, the course_end_date should always be in past
+            assert credit_state["course_end_date"] < datetime.now()
+
+            exception_log.exception.assert_called_once_with(
+                "Could not get name and end_date for course %s, This happened because we were unable to "
+                "get/create CourseOverview object for the course. It's possible that the Course has been deleted.",
+                str(self.course.id))
+
     def test_set_status_non_credit(self):
         """
         assert that we can still try to update a credit status but return quickly if