From 6ab42849b7378231c03c161baaf38aba3cbedd8f Mon Sep 17 00:00:00 2001 From: Alex Dusenbery Date: Tue, 7 Jan 2020 17:01:38 -0500 Subject: [PATCH] CR-1640 | Makes course_api.api.get_due_dates() a little more defensive. --- lms/djangoapps/course_api/api.py | 17 ++++- lms/djangoapps/course_api/tests/test_api.py | 79 ++++++++++++++++++++- 2 files changed, 90 insertions(+), 6 deletions(-) diff --git a/lms/djangoapps/course_api/api.py b/lms/djangoapps/course_api/api.py index feb6493007..76c4240690 100644 --- a/lms/djangoapps/course_api/api.py +++ b/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, }) diff --git a/lms/djangoapps/course_api/tests/test_api.py b/lms/djangoapps/course_api/tests/test_api.py index e1fbb100f0..dda51d7e7c 100644 --- a/lms/djangoapps/course_api/tests/test_api.py +++ b/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