CR-1640 | Makes course_api.api.get_due_dates() a little more defensive.

lms/djangoapps/course_api/api.py

@@ -1,7 +1,7 @@
 """
 Course API
 """
-
+import logging
 
 from edx_when.api import get_dates_for_course
 from django.conf import settings
@@ -19,10 +19,17 @@ from lms.djangoapps.courseware.courses import (
 )
 from openedx.core.lib.api.view_utils import LazySequence
 from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.exceptions import ItemNotFoundError
 
 from .permissions import can_view_courses_for_username
 
 
+logger = logging.getLogger(__name__)  # pylint: disable=invalid-name
+
+
+UNKNOWN_BLOCK_DISPLAY_NAME = 'UNKNOWN'
+
+
 def get_effective_user(requesting_user, target_username):
     """
     Get the user we want to view information on behalf of.
@@ -178,14 +185,18 @@ def get_due_dates(request, course_key, user):
     due_dates = []
     for (block_key, date_type), date in six.iteritems(dates):
         if date_type == 'due':
-            block = store.get_item(block_key)
+            try:
+                block_display_name = store.get_item(block_key).display_name
+            except ItemNotFoundError:
+                logger.exception('Failed to get block for due date item with key: {}'.format(block_key))
+                block_display_name = UNKNOWN_BLOCK_DISPLAY_NAME
 
             # get url to the block in the course
             block_url = reverse('jump_to', args=[course_key, block_key])
             block_url = request.build_absolute_uri(block_url)
 
             due_dates.append({
-                'name': block.display_name,
+                'name': block_display_name,
                 'url': block_url,
                 'date': date,
             })

lms/djangoapps/course_api/tests/test_api.py

@@ -2,21 +2,23 @@
 Test for course API
 """
 
-
+from datetime import datetime, timedelta
 from hashlib import md5
 
 from django.contrib.auth.models import AnonymousUser
 from django.http import Http404
+import mock
 from opaque_keys.edx.keys import CourseKey
 from rest_framework.exceptions import PermissionDenied
 from rest_framework.request import Request
 from rest_framework.test import APIRequestFactory
 
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
+from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase
-from xmodule.modulestore.tests.factories import check_mongo_calls
+from xmodule.modulestore.tests.factories import ItemFactory, check_mongo_calls
 
-from ..api import course_detail, list_courses
+from ..api import UNKNOWN_BLOCK_DISPLAY_NAME, course_detail, get_due_dates, list_courses
 from .mixins import CourseApiFactoryMixin
 
 
@@ -238,3 +240,74 @@ class TestGetCourseListExtras(CourseListTestMixin, ModuleStoreTestCase):
         self.create_course(visible_to_staff_only=True)
         courses = self._make_api_call(self.staff_user, self.staff_user)
         self.verify_courses(courses)
+
+
+class TestGetCourseDates(CourseDetailTestMixin, SharedModuleStoreTestCase):
+    """
+    Test get_due_dates function
+    """
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.course = cls.create_course()
+        cls.staff_user = cls.create_user("staff", is_staff=True)
+        cls.today = datetime.utcnow()
+        cls.yesterday = cls.today - timedelta(days=1)
+        cls.tomorrow = cls.today + timedelta(days=1)
+
+        cls.section_1 = ItemFactory.create(
+            category='chapter',
+            start=cls.yesterday,
+            due=cls.tomorrow,
+            parent=cls.course,
+            display_name='section 1'
+        )
+
+        cls.subsection_1 = ItemFactory.create(
+            category='sequential',
+            parent=cls.section_1,
+            display_name='subsection 1'
+        )
+
+    def test_get_due_dates(self):
+        request = mock.Mock()
+
+        mock_path = 'lms.djangoapps.course_api.api.get_dates_for_course'
+        with mock.patch(mock_path) as mock_get_dates:
+            mock_get_dates.return_value = {
+                (self.section_1.location, 'due'): self.section_1.due.strftime('%Y-%m-%dT%H:%M:%SZ'),
+                (self.section_1.location, 'start'): self.section_1.start.strftime('%Y-%m-%dT%H:%M:%SZ'),
+            }
+
+            expected_due_dates = [
+                {
+                    'name': self.section_1.display_name,
+                    'url': request.build_absolute_uri.return_value,
+                    'date': self.tomorrow.strftime('%Y-%m-%dT%H:%M:%SZ'),
+                },
+            ]
+            actual_due_dates = get_due_dates(request, self.course.id, self.staff_user)
+            assert expected_due_dates == actual_due_dates
+
+    def test_get_due_dates_error_fetching_block(self):
+        request = mock.Mock()
+
+        mock_path = 'lms.djangoapps.course_api.api.'
+        with mock.patch(mock_path + 'get_dates_for_course') as mock_get_dates:
+            with mock.patch(mock_path + 'modulestore') as mock_modulestore:
+                mock_modulestore.return_value.get_item.side_effect = ItemNotFoundError('whatever')
+                mock_get_dates.return_value = {
+                    (self.section_1.location, 'due'): self.section_1.due.strftime('%Y-%m-%dT%H:%M:%SZ'),
+                    (self.section_1.location, 'start'): self.section_1.start.strftime('%Y-%m-%dT%H:%M:%SZ'),
+                }
+
+                expected_due_dates = [
+                    {
+                        'name': UNKNOWN_BLOCK_DISPLAY_NAME,
+                        'url': request.build_absolute_uri.return_value,
+                        'date': self.tomorrow.strftime('%Y-%m-%dT%H:%M:%SZ'),
+                    },
+                ]
+                actual_due_dates = get_due_dates(request, self.course.id, self.staff_user)
+                assert expected_due_dates == actual_due_dates