From 8db2dd47dd818a72c159d415e3abec0b97bad8ab Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Tue, 5 Jun 2018 21:25:21 -0400 Subject: [PATCH] Course Validation and Course Quality APIs --- .../tests/{test_views.py => test_import.py} | 6 +- .../contentstore/api/tests/test_quality.py | 142 ++++++++++ .../contentstore/api/tests/test_validation.py | 102 +++++++ cms/djangoapps/contentstore/api/urls.py | 10 +- .../contentstore/api/views/__init__.py | 0 .../api/{views.py => views/course_import.py} | 54 ++-- .../contentstore/api/views/course_quality.py | 252 ++++++++++++++++++ .../api/views/course_validation.py | 178 +++++++++++++ .../contentstore/api/views/utils.py | 47 ++++ .../contentstore/views/certificates.py | 26 +- common/lib/xmodule/xmodule/graders.py | 7 + lms/djangoapps/grades/api/v1/views.py | 8 +- lms/djangoapps/grades/api/views.py | 21 +- openedx/core/djangoapps/util/forms.py | 32 ++- openedx/core/lib/api/view_utils.py | 35 ++- 15 files changed, 830 insertions(+), 90 deletions(-) rename cms/djangoapps/contentstore/api/tests/{test_views.py => test_import.py} (97%) create mode 100644 cms/djangoapps/contentstore/api/tests/test_quality.py create mode 100644 cms/djangoapps/contentstore/api/tests/test_validation.py create mode 100644 cms/djangoapps/contentstore/api/views/__init__.py rename cms/djangoapps/contentstore/api/{views.py => views/course_import.py} (75%) create mode 100644 cms/djangoapps/contentstore/api/views/course_quality.py create mode 100644 cms/djangoapps/contentstore/api/views/course_validation.py create mode 100644 cms/djangoapps/contentstore/api/views/utils.py diff --git a/cms/djangoapps/contentstore/api/tests/test_views.py b/cms/djangoapps/contentstore/api/tests/test_import.py similarity index 97% rename from cms/djangoapps/contentstore/api/tests/test_views.py rename to cms/djangoapps/contentstore/api/tests/test_import.py index 710663498d..5c9f3f34b6 100644 --- a/cms/djangoapps/contentstore/api/tests/test_views.py +++ b/cms/djangoapps/contentstore/api/tests/test_import.py @@ -2,19 +2,15 @@ Tests for the course import API views """ import os -import shutil import tarfile import tempfile -from datetime import datetime -from urllib import urlencode from django.urls import reverse from path import Path as path -from mock import patch from rest_framework import status from rest_framework.test import APITestCase -from lms.djangoapps.courseware.tests.factories import GlobalStaffFactory, StaffFactory +from lms.djangoapps.courseware.tests.factories import StaffFactory from student.tests.factories import UserFactory from user_tasks.models import UserTaskStatus from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, SharedModuleStoreTestCase diff --git a/cms/djangoapps/contentstore/api/tests/test_quality.py b/cms/djangoapps/contentstore/api/tests/test_quality.py new file mode 100644 index 0000000000..e7dba1c458 --- /dev/null +++ b/cms/djangoapps/contentstore/api/tests/test_quality.py @@ -0,0 +1,142 @@ +""" +Tests for the course import API views +""" +from django.core.urlresolvers import reverse +from rest_framework import status +from rest_framework.test import APITestCase + +from lms.djangoapps.courseware.tests.factories import StaffFactory +from student.tests.factories import UserFactory +from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory + + +class CourseQualityViewTest(SharedModuleStoreTestCase, APITestCase): + """ + Test importing courses via a RESTful API (POST method only) + """ + MODULESTORE = TEST_DATA_SPLIT_MODULESTORE + + @classmethod + def setUpClass(cls): + super(CourseQualityViewTest, cls).setUpClass() + + cls.course = CourseFactory.create(display_name='test course', run="Testing_course") + cls.course_key = cls.course.id + + cls.password = 'test' + cls.student = UserFactory(username='dummy', password=cls.password) + cls.staff = StaffFactory(course_key=cls.course.id, password=cls.password) + + cls.initialize_course(cls.course) + + @classmethod + def initialize_course(cls, course): + course.self_paced = True + cls.store.update_item(course, cls.staff.id) + + section = ItemFactory.create( + parent_location=course.location, + category="chapter", + ) + subsection1 = ItemFactory.create( + parent_location=section.location, + category="sequential", + ) + unit1 = ItemFactory.create( + parent_location=subsection1.location, + category="vertical", + ) + ItemFactory.create( + parent_location=unit1.location, + category="video", + ) + ItemFactory.create( + parent_location=unit1.location, + category="problem", + ) + + subsection2 = ItemFactory.create( + parent_location=section.location, + category="sequential", + ) + unit2 = ItemFactory.create( + parent_location=subsection2.location, + category="vertical", + ) + unit3 = ItemFactory.create( + parent_location=subsection2.location, + category="vertical", + ) + ItemFactory.create( + parent_location=unit3.location, + category="video", + ) + ItemFactory.create( + parent_location=unit3.location, + category="video", + ) + + def get_url(self, course_id): + """ + Helper function to create the url + """ + return reverse( + 'courses_api:course_quality', + kwargs={ + 'course_id': course_id + } + ) + + def test_student_fails(self): + self.client.login(username=self.student.username, password=self.password) + resp = self.client.get(self.get_url(self.course_key)) + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + + def test_staff_succeeds(self): + self.client.login(username=self.staff.username, password=self.password) + resp = self.client.get(self.get_url(self.course_key), {'all': 'true'}) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + expected_data = { + 'units': { + 'num_blocks': { + 'max': 2, + 'mean': 1.0, + 'median': 2.0, + 'mode': 2.0, + 'min': 0, + }, + 'total_visible': 3, + }, + 'videos': { + 'durations': { + 'max': None, + 'mean': None, + 'median': None, + 'mode': None, + 'min': None, + }, + 'num_mobile_encoded': 0, + 'num_with_val_id': 0, + 'total_number': 3, + }, + 'sections': { + 'number_with_highlights': 0, + 'total_visible': 1, + 'total_number': 1, + 'highlights_enabled': False, + }, + 'subsections': { + 'num_with_one_block_type': 1, + 'num_block_types': { + 'max': 2, + 'mean': 2.0, + 'median': 2.0, + 'mode': 1.0, + 'min': 1, + }, + 'total_visible': 2, + }, + 'is_self_paced': True, + } + self.assertDictEqual(resp.data, expected_data) diff --git a/cms/djangoapps/contentstore/api/tests/test_validation.py b/cms/djangoapps/contentstore/api/tests/test_validation.py new file mode 100644 index 0000000000..acaa4dc0c6 --- /dev/null +++ b/cms/djangoapps/contentstore/api/tests/test_validation.py @@ -0,0 +1,102 @@ +""" +Tests for the course import API views +""" +from datetime import datetime +from django.core.urlresolvers import reverse +from rest_framework import status +from rest_framework.test import APITestCase + +from lms.djangoapps.courseware.tests.factories import StaffFactory +from student.tests.factories import UserFactory +from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory + + +class CourseValidationViewTest(SharedModuleStoreTestCase, APITestCase): + """ + Test importing courses via a RESTful API (POST method only) + """ + MODULESTORE = TEST_DATA_SPLIT_MODULESTORE + + @classmethod + def setUpClass(cls): + super(CourseValidationViewTest, cls).setUpClass() + + cls.course = CourseFactory.create(display_name='test course', run="Testing_course") + cls.course_key = cls.course.id + + cls.password = 'test' + cls.student = UserFactory(username='dummy', password=cls.password) + cls.staff = StaffFactory(course_key=cls.course.id, password=cls.password) + + cls.initialize_course(cls.course) + + @classmethod + def initialize_course(cls, course): + course.start = datetime.now() + course.self_paced = True + cls.store.update_item(course, cls.staff.id) + + update_key = course.id.make_usage_key('course_info', 'updates') + cls.store.create_item( + cls.staff.id, + update_key.course_key, + update_key.block_type, + block_id=update_key.block_id, + fields=dict(data=u"

  1. Date

    Hello world!
"), + ) + + section = ItemFactory.create( + parent_location=course.location, + category="chapter", + ) + ItemFactory.create( + parent_location=section.location, + category="sequential", + ) + + def get_url(self, course_id): + """ + Helper function to create the url + """ + return reverse( + 'courses_api:course_validation', + kwargs={ + 'course_id': course_id, + } + ) + + def test_student_fails(self): + self.client.login(username=self.student.username, password=self.password) + resp = self.client.get(self.get_url(self.course_key)) + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + + def test_staff_succeeds(self): + self.client.login(username=self.staff.username, password=self.password) + resp = self.client.get(self.get_url(self.course_key), {'all': 'true'}) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + expected_data = { + 'assignments': { + 'num_with_dates_before_end': 0, + 'num_with_dates': 0, + 'total_visible': 1, + 'num_with_dates_after_start': 0, + 'total_number': 1, + }, + 'dates': { + 'has_start_date': True, + 'has_end_date': False, + }, + 'updates': { + 'has_update': True, + }, + 'certificates': { + 'is_activated': False, + 'has_certificate': False, + }, + 'grades': { + 'sum_of_weights': 1.0, + }, + 'is_self_paced': True, + } + self.assertDictEqual(resp.data, expected_data) diff --git a/cms/djangoapps/contentstore/api/urls.py b/cms/djangoapps/contentstore/api/urls.py index d758aa5ed3..97cec4c1ee 100644 --- a/cms/djangoapps/contentstore/api/urls.py +++ b/cms/djangoapps/contentstore/api/urls.py @@ -1,10 +1,14 @@ -""" Course Import API URLs. """ +""" Course API URLs. """ from django.conf import settings from django.conf.urls import url -from cms.djangoapps.contentstore.api import views +from cms.djangoapps.contentstore.api.views import course_import, course_validation, course_quality urlpatterns = [ url(r'^v0/import/{course_id}/$'.format(course_id=settings.COURSE_ID_PATTERN,), - views.CourseImportView.as_view(), name='course_import'), + course_import.CourseImportView.as_view(), name='course_import'), + url(r'^v1/validation/{course_id}/$'.format(course_id=settings.COURSE_ID_PATTERN,), + course_validation.CourseValidationView.as_view(), name='course_validation'), + url(r'^v1/quality/{course_id}/$'.format(course_id=settings.COURSE_ID_PATTERN,), + course_quality.CourseQualityView.as_view(), name='course_quality'), ] diff --git a/cms/djangoapps/contentstore/api/views/__init__.py b/cms/djangoapps/contentstore/api/views/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cms/djangoapps/contentstore/api/views.py b/cms/djangoapps/contentstore/api/views/course_import.py similarity index 75% rename from cms/djangoapps/contentstore/api/views.py rename to cms/djangoapps/contentstore/api/views/course_import.py index aa4f20d087..639b504bfd 100644 --- a/cms/djangoapps/contentstore/api/views.py +++ b/cms/djangoapps/contentstore/api/views/course_import.py @@ -1,4 +1,6 @@ -""" API v0 views. """ +""" +APIs related to Course Import. +""" import base64 import logging import os @@ -9,19 +11,19 @@ from six import text_type from django.conf import settings from django.core.files import File -from opaque_keys.edx.keys import CourseKey from rest_framework import status from rest_framework.exceptions import AuthenticationFailed from rest_framework.generics import GenericAPIView from rest_framework.response import Response from user_tasks.models import UserTaskStatus -from student.auth import has_course_author_access - from contentstore.storage import course_import_export_storage from contentstore.tasks import CourseImportTask, import_olx from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes +from .utils import course_author_access_required + + log = logging.getLogger(__name__) @@ -106,39 +108,30 @@ class CourseImportView(CourseImportExportViewMixin, GenericAPIView): # does not specify a serializer class. exclude_from_schema = True - def post(self, request, course_id): + @course_author_access_required + def post(self, request, course_key): """ Kicks off an asynchronous course import and returns an ID to be used to check the task's status """ - - courselike_key = CourseKey.from_string(course_id) - if not has_course_author_access(request.user, courselike_key): - return self.make_error_response( - status_code=status.HTTP_403_FORBIDDEN, - developer_message='The user requested does not have the required permissions.', - error_code='user_mismatch' - ) try: if 'course_data' not in request.FILES: - return self.make_error_response( + raise self.api_error( status_code=status.HTTP_400_BAD_REQUEST, developer_message='Missing required parameter', error_code='internal_error', - field_errors={'course_data': '"course_data" parameter is required, and must be a .tar.gz file'} ) filename = request.FILES['course_data'].name if not filename.endswith('.tar.gz'): - return self.make_error_response( + raise self.api_error( status_code=status.HTTP_400_BAD_REQUEST, developer_message='Parameter in the wrong format', error_code='internal_error', - field_errors={'course_data': '"course_data" parameter is required, and must be a .tar.gz file'} ) - course_dir = path(settings.GITHUB_REPO_ROOT) / base64.urlsafe_b64encode(repr(courselike_key)) + course_dir = path(settings.GITHUB_REPO_ROOT) / base64.urlsafe_b64encode(repr(course_key)) temp_filepath = course_dir / filename - if not course_dir.isdir(): # pylint: disable=no-value-for-parameter + if not course_dir.isdir(): os.mkdir(course_dir) log.debug('importing course to {0}'.format(temp_filepath)) @@ -146,46 +139,41 @@ class CourseImportView(CourseImportExportViewMixin, GenericAPIView): for chunk in request.FILES['course_data'].chunks(): temp_file.write(chunk) - log.info("Course import %s: Upload complete", courselike_key) + log.info("Course import %s: Upload complete", course_key) with open(temp_filepath, 'rb') as local_file: django_file = File(local_file) storage_path = course_import_export_storage.save(u'olx_import/' + filename, django_file) async_result = import_olx.delay( - request.user.id, text_type(courselike_key), storage_path, filename, request.LANGUAGE_CODE) + request.user.id, text_type(course_key), storage_path, filename, request.LANGUAGE_CODE) return Response({ 'task_id': async_result.task_id }) except Exception as e: - return self.make_error_response( + log.exception(str(e)) + raise self.api_error( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, developer_message=str(e), error_code='internal_error' ) - def get(self, request, course_id): + @course_author_access_required + def get(self, request, course_key): """ Check the status of the specified task """ - - courselike_key = CourseKey.from_string(course_id) - if not has_course_author_access(request.user, courselike_key): - return self.make_error_response( - status_code=status.HTTP_403_FORBIDDEN, - developer_message='The user requested does not have the required permissions.', - error_code='user_mismatch' - ) try: task_id = request.GET['task_id'] filename = request.GET['filename'] - args = {u'course_key_string': course_id, u'archive_name': filename} + args = {u'course_key_string': str(course_key), u'archive_name': filename} name = CourseImportTask.generate_name(args) task_status = UserTaskStatus.objects.filter(name=name, task_id=task_id).first() return Response({ 'state': task_status.state }) except Exception as e: - return self.make_error_response( + log.exception(str(e)) + raise self.api_error( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, developer_message=str(e), error_code='internal_error' diff --git a/cms/djangoapps/contentstore/api/views/course_quality.py b/cms/djangoapps/contentstore/api/views/course_quality.py new file mode 100644 index 0000000000..e2ecfc841a --- /dev/null +++ b/cms/djangoapps/contentstore/api/views/course_quality.py @@ -0,0 +1,252 @@ +# pylint: disable=missing-docstring +import logging +import numpy as np +from scipy import stats +from rest_framework.generics import GenericAPIView +from rest_framework.response import Response + +from edxval.api import get_videos_for_course +from openedx.core.djangoapps.request_cache.middleware import request_cached +from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes +from openedx.core.lib.graph_traversals import traverse_pre_order +from xmodule.modulestore.django import modulestore + +from .utils import get_bool_param, course_author_access_required + +log = logging.getLogger(__name__) + + +@view_auth_classes() +class CourseQualityView(DeveloperErrorViewMixin, GenericAPIView): + """ + **Use Case** + + **Example Requests** + + GET /api/courses/v1/quality/{course_id}/ + + **GET Parameters** + + A GET request may include the following parameters. + + * all + * sections + * subsections + * units + * videos + * exclude_graded (boolean) - whether to exclude graded subsections in the subsections and units information. + + **GET Response Values** + + The HTTP 200 response has the following values. + + * is_self_paced - whether the course is self-paced. + * sections + * total_number - number of sections in the course. + * total_visible - number of sections visible to learners in the course. + * number_with_highlights - number of sections that have at least one highlight entered. + * highlights_enabled - whether highlights are enabled in the course. + * subsections + * total_visible - number of subsections visible to learners in the course. + * num_with_one_block_type - number of visible subsections containing only one type of block. + * num_block_types - statistics for number of block types across all visible subsections. + * min + * max + * mean + * median + * mode + * units + * total_visible - number of units visible to learners in the course. + * num_blocks - statistics for number of block across all visible units. + * min + * max + * mean + * median + * mode + * videos + * total_number - number of video blocks in the course. + * num_with_val_id - number of video blocks that include video pipeline IDs. + * num_mobile_encoded - number of videos encoded through the video pipeline. + * durations - statistics for video duration across all videos encoded through the video pipeline. + * min + * max + * mean + * median + * mode + + """ + @course_author_access_required + def get(self, request, course_key): + """ + Returns validation information for the given course. + """ + all_requested = get_bool_param(request, 'all', False) + + store = modulestore() + with store.bulk_operations(course_key): + course = store.get_course(course_key, depth=self._required_course_depth(request, all_requested)) + + response = dict( + is_self_paced=course.self_paced, + ) + if get_bool_param(request, 'sections', all_requested): + response.update( + sections=self._sections_quality(course) + ) + if get_bool_param(request, 'subsections', all_requested): + response.update( + subsections=self._subsections_quality(course, request) + ) + if get_bool_param(request, 'units', all_requested): + response.update( + units=self._units_quality(course, request) + ) + if get_bool_param(request, 'videos', all_requested): + response.update( + videos=self._videos_quality(course) + ) + + return Response(response) + + def _required_course_depth(self, request, all_requested): + if get_bool_param(request, 'units', all_requested): + # The num_blocks metric for "units" requires retrieving all blocks in the graph. + return None + elif get_bool_param(request, 'subsections', all_requested): + # The num_block_types metric for "subsections" requires retrieving all blocks in the graph. + return None + elif get_bool_param(request, 'sections', all_requested): + return 1 + else: + return 0 + + def _sections_quality(self, course): + sections, visible_sections = self._get_sections(course) + sections_with_highlights = [s for s in visible_sections if s.highlights] + return dict( + total_number=len(sections), + total_visible=len(visible_sections), + number_with_highlights=len(sections_with_highlights), + highlights_enabled=course.highlights_enabled_for_messaging, + ) + + def _subsections_quality(self, course, request): + subsection_unit_dict = self._get_subsections_and_units(course, request) + num_block_types_per_subsection_dict = {} + for subsection_key, unit_dict in subsection_unit_dict.iteritems(): + leaf_block_types_in_subsection = ( + unit_info['leaf_block_types'] + for unit_info in unit_dict.itervalues() + ) + num_block_types_per_subsection_dict[subsection_key] = len(set().union(*leaf_block_types_in_subsection)) + + return dict( + total_visible=len(num_block_types_per_subsection_dict), + num_with_one_block_type=list(num_block_types_per_subsection_dict.itervalues()).count(1), + num_block_types=self._stats_dict(list(num_block_types_per_subsection_dict.itervalues())), + ) + + def _units_quality(self, course, request): + subsection_unit_dict = self._get_subsections_and_units(course, request) + num_leaf_blocks_per_unit = [ + unit_info['num_leaf_blocks'] + for unit_dict in subsection_unit_dict.itervalues() + for unit_info in unit_dict.itervalues() + ] + return dict( + total_visible=len(num_leaf_blocks_per_unit), + num_blocks=self._stats_dict(num_leaf_blocks_per_unit), + ) + + def _videos_quality(self, course): + video_blocks_in_course = modulestore().get_items(course.id, qualifiers={'category': 'video'}) + videos_in_val = list(get_videos_for_course(course.id)) + video_durations = [video['duration'] for video in videos_in_val] + + return dict( + total_number=len(video_blocks_in_course), + num_mobile_encoded=len(videos_in_val), + num_with_val_id=len([v for v in video_blocks_in_course if v.edx_video_id]), + durations=self._stats_dict(video_durations), + ) + + @request_cached + def _get_subsections_and_units(self, course, request): + """ + Returns {subsection_key: {unit_key: {num_leaf_blocks: <>, leaf_block_types: set(<>) }}} + for all visible subsections and units. + """ + _, visible_sections = self._get_sections(course) + subsection_dict = {} + for section in visible_sections: + visible_subsections = self._get_visible_children(section) + + if get_bool_param(request, 'exclude_graded', False): + visible_subsections = [s for s in visible_subsections if not s.graded] + + for subsection in visible_subsections: + unit_dict = {} + visible_units = self._get_visible_children(subsection) + + for unit in visible_units: + leaf_blocks = self._get_leaf_blocks(unit) + unit_dict[unit.location] = dict( + num_leaf_blocks=len(leaf_blocks), + leaf_block_types=set(block.location.block_type for block in leaf_blocks), + ) + + subsection_dict[subsection.location] = unit_dict + return subsection_dict + + @request_cached + def _get_sections(self, course): + return self._get_all_children(course) + + def _get_all_children(self, parent): + store = modulestore() + children = [store.get_item(child_usage_key) for child_usage_key in self._get_children(parent)] + visible_children = [ + c for c in children + if not c.visible_to_staff_only and not c.hide_from_toc + ] + return children, visible_children + + def _get_visible_children(self, parent): + _, visible_chidren = self._get_all_children(parent) + return visible_chidren + + def _get_children(self, parent): + if not hasattr(parent, 'children'): + return [] + else: + return parent.children + + def _get_leaf_blocks(self, unit): + def leaf_filter(block): + return ( + block.location.block_type not in ('chapter', 'sequential', 'vertical') and + len(self._get_children(block)) == 0 + ) + + return [ + block for block in + traverse_pre_order(unit, self._get_visible_children, leaf_filter) + ] + + def _stats_dict(self, data): + if not data: + return dict( + min=None, + max=None, + mean=None, + median=None, + mode=None, + ) + else: + return dict( + min=min(data), + max=max(data), + mean=np.around(np.mean(data)), + median=np.around(np.median(data)), + mode=stats.mode(data, axis=None)[0][0], + ) diff --git a/cms/djangoapps/contentstore/api/views/course_validation.py b/cms/djangoapps/contentstore/api/views/course_validation.py new file mode 100644 index 0000000000..f6c68e9e17 --- /dev/null +++ b/cms/djangoapps/contentstore/api/views/course_validation.py @@ -0,0 +1,178 @@ +# pylint: disable=missing-docstring +import logging +from rest_framework.generics import GenericAPIView +from rest_framework.response import Response + +from contentstore.course_info_model import get_course_updates +from contentstore.views.certificates import CertificateManager +from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes +from xmodule.modulestore.django import modulestore + +from .utils import get_bool_param, course_author_access_required + + +log = logging.getLogger(__name__) + + +@view_auth_classes() +class CourseValidationView(DeveloperErrorViewMixin, GenericAPIView): + """ + **Use Case** + + **Example Requests** + + GET /api/courses/v1/validation/{course_id}/ + + **GET Parameters** + + A GET request may include the following parameters. + + * all + * dates + * assignments + * grades + * certificates + * updates + + **GET Response Values** + + The HTTP 200 response has the following values. + + * is_self_paced - whether the course is self-paced. + * dates + * has_start_date - whether the start date is set on the course. + * has_end_date - whether the end date is set on the course. + * assignments + * total_number - total number of assignments in the course. + * total_visible - number of assignments visible to learners in the course. + * num_with_dates - number of assignments with due dates. + * num_with_dates_after_start - number of assignments with due dates after the start date. + * num_with_dates_before_end - number of assignments with due dates before the end date. + * grades + * sum_of_weights - sum of weights for all assignments in the course (valid ones should equal 1). + * certificates + * is_activated - whether the certificate is activated for the course. + * has_certificate - whether the course has a certificate. + * updates + * has_update - whether at least one course update exists. + + """ + @course_author_access_required + def get(self, request, course_key): + """ + Returns validation information for the given course. + """ + all_requested = get_bool_param(request, 'all', False) + + store = modulestore() + with store.bulk_operations(course_key): + course = store.get_course(course_key, depth=self._required_course_depth(request, all_requested)) + + response = dict( + is_self_paced=course.self_paced, + ) + if get_bool_param(request, 'dates', all_requested): + response.update( + dates=self._dates_validation(course) + ) + if get_bool_param(request, 'assignments', all_requested): + response.update( + assignments=self._assignments_validation(course) + ) + if get_bool_param(request, 'grades', all_requested): + response.update( + grades=self._grades_validation(course) + ) + if get_bool_param(request, 'certificates', all_requested): + response.update( + certificates=self._certificates_validation(course) + ) + if get_bool_param(request, 'updates', all_requested): + response.update( + updates=self._updates_validation(course, request) + ) + + return Response(response) + + def _required_course_depth(self, request, all_requested): + if get_bool_param(request, 'assignments', all_requested): + return 2 + else: + return 0 + + def _dates_validation(self, course): + return dict( + has_start_date=self._has_start_date(course), + has_end_date=course.end is not None, + ) + + def _assignments_validation(self, course): + assignments, visible_assignments = self._get_assignments(course) + assignments_with_dates = [a for a in visible_assignments if a.due] + + num_with_dates = len(assignments_with_dates) + num_with_dates_after_start = ( + len([a for a in assignments_with_dates if a.due > course.start]) + if self._has_start_date(course) + else 0 + ) + num_with_dates_before_end = ( + len([a for a in assignments_with_dates if a.due < course.end]) + if course.end + else 0 + ) + + return dict( + total_number=len(assignments), + total_visible=len(visible_assignments), + num_with_dates=num_with_dates, + num_with_dates_after_start=num_with_dates_after_start, + num_with_dates_before_end=num_with_dates_before_end, + ) + + def _grades_validation(self, course): + sum_of_weights = course.grader.sum_of_weights + return dict( + sum_of_weights=sum_of_weights, + ) + + def _certificates_validation(self, course): + is_activated, certificates = CertificateManager.is_activated(course) + return dict( + is_activated=is_activated, + has_certificate=len(certificates) > 0, + ) + + def _updates_validation(self, course, request): + updates_usage_key = course.id.make_usage_key('course_info', 'updates') + updates = get_course_updates(updates_usage_key, provided_id=None, user_id=request.user.id) + return dict( + has_update=len(updates) > 0, + ) + + def _get_assignments(self, course): + store = modulestore() + sections = [store.get_item(section_usage_key) for section_usage_key in course.children] + assignments = [ + store.get_item(assignment_usage_key) + for section in sections + for assignment_usage_key in section.children + ] + + visible_sections = [ + s for s in sections + if not s.visible_to_staff_only and not s.hide_from_toc + ] + assignments_in_visible_sections = [ + store.get_item(assignment_usage_key) + for visible_section in visible_sections + for assignment_usage_key in visible_section.children + ] + visible_assignments = [ + a for a in assignments_in_visible_sections + if not a.visible_to_staff_only + ] + return assignments, visible_assignments + + def _has_start_date(self, course): + return not course.start_date_is_still_default diff --git a/cms/djangoapps/contentstore/api/views/utils.py b/cms/djangoapps/contentstore/api/views/utils.py new file mode 100644 index 0000000000..589ce3de04 --- /dev/null +++ b/cms/djangoapps/contentstore/api/views/utils.py @@ -0,0 +1,47 @@ +""" +Common utilities for Contentstore APIs. +""" +from rest_framework import status + +from opaque_keys.edx.keys import CourseKey +from openedx.core.djangoapps.util.forms import to_bool +from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin +from student.auth import has_course_author_access + + +def get_bool_param(request, param_name, default): + param_value = request.query_params.get(param_name, None) + bool_value = to_bool(param_value) + if bool_value is None: + return default + else: + return bool_value + + +def course_author_access_required(view): + """ + Ensure the user making the API request has course author access to the given course. + + This decorator parses the course_id parameter, checks course access, and passes + the parsed course_key to the view as a parameter. It will raise a + 403 error if the user does not have author access. + + Usage:: + @course_author_access_required + def my_view(request, course_key): + # Some functionality ... + """ + def _wrapper_view(self, request, course_id, *args, **kwargs): + """ + Checks for course author access for the given course by the requesting user. + Calls the view function if has access, otherwise raises a 403. + """ + course_key = CourseKey.from_string(course_id) + if not has_course_author_access(request.user, course_key): + raise DeveloperErrorViewMixin.api_error( + status_code=status.HTTP_403_FORBIDDEN, + developer_message='The requesting user does not have course author permissions.', + error_code='user_permissions', + ) + return view(self, request, course_key, *args, **kwargs) + return _wrapper_view diff --git a/cms/djangoapps/contentstore/views/certificates.py b/cms/djangoapps/contentstore/views/certificates.py index fd2e17d756..d2ed27ae50 100644 --- a/cms/djangoapps/contentstore/views/certificates.py +++ b/cms/djangoapps/contentstore/views/certificates.py @@ -157,6 +157,22 @@ class CertificateManager(object): if not certificate_data.get("name"): raise CertificateValidationError(_("must have name of the certificate")) + @staticmethod + def is_activated(course): + """ + Returns whether certificates are activated for the given course, + along with the certificates. + """ + is_active = False + certificates = None + if settings.FEATURES.get('CERTIFICATES_HTML_VIEW', False): + certificates = CertificateManager.get_certificates(course) + # we are assuming only one certificate in certificates collection. + for certificate in certificates: + is_active = certificate.get('is_active', False) + break + return is_active, certificates + @staticmethod def get_used_ids(course): """ @@ -392,14 +408,8 @@ def certificates_list_handler(request, course_key_string): ) else: certificate_web_view_url = None - certificates = None - is_active = False - if settings.FEATURES.get('CERTIFICATES_HTML_VIEW', False): - certificates = CertificateManager.get_certificates(course) - # we are assuming only one certificate in certificates collection. - for certificate in certificates: - is_active = certificate.get('is_active', False) - break + + is_active, certificates = CertificateManager.is_activated(course) return render_to_response('certificates.html', { 'context_course': course, diff --git a/common/lib/xmodule/xmodule/graders.py b/common/lib/xmodule/xmodule/graders.py index f1f11b7e1e..f04a4bcd9e 100644 --- a/common/lib/xmodule/xmodule/graders.py +++ b/common/lib/xmodule/xmodule/graders.py @@ -262,6 +262,13 @@ class WeightedSubsectionsGrader(CourseGrader): def __init__(self, subgraders): self.subgraders = subgraders + @property + def sum_of_weights(self): + sum = 0 + for _, _, weight in self.subgraders: + sum += weight + return sum + def grade(self, grade_sheet, generate_random_scores=False): total_percent = 0.0 section_breakdown = [] diff --git a/lms/djangoapps/grades/api/v1/views.py b/lms/djangoapps/grades/api/v1/views.py index f9696c3bbd..36c63396a2 100644 --- a/lms/djangoapps/grades/api/v1/views.py +++ b/lms/djangoapps/grades/api/v1/views.py @@ -168,14 +168,14 @@ class CourseGradesView(GradeViewMixin, GenericAPIView): try: course_key = CourseKey.from_string(course_id) except InvalidKeyError: - return self.make_error_response( + raise self.api_error( status_code=status.HTTP_404_NOT_FOUND, developer_message='The provided course key cannot be parsed.', error_code='invalid_course_key' ) if not CourseOverview.get_from_id_if_exists(course_key): - return self.make_error_response( + raise self.api_error( status_code=status.HTTP_404_NOT_FOUND, developer_message="Requested grade for unknown course {course}".format(course=course_id), error_code='course_does_not_exist' @@ -186,13 +186,13 @@ class CourseGradesView(GradeViewMixin, GenericAPIView): try: return self._get_single_user_grade(request, course_key) except USER_MODEL.DoesNotExist: - return self.make_error_response( + raise self.api_error( status_code=status.HTTP_404_NOT_FOUND, developer_message='The user matching the requested username does not exist.', error_code='user_does_not_exist' ) except CourseEnrollment.DoesNotExist: - return self.make_error_response( + raise self.api_error( status_code=status.HTTP_404_NOT_FOUND, developer_message='The user matching the requested username is not enrolled in this course', error_code='user_not_enrolled' diff --git a/lms/djangoapps/grades/api/views.py b/lms/djangoapps/grades/api/views.py index d16a4b627f..a54fe58ac5 100644 --- a/lms/djangoapps/grades/api/views.py +++ b/lms/djangoapps/grades/api/views.py @@ -35,7 +35,7 @@ class GradeViewMixin(DeveloperErrorViewMixin): try: course_key = CourseKey.from_string(course_key_string) except InvalidKeyError: - return self.make_error_response( + raise self.api_error( status_code=status.HTTP_404_NOT_FOUND, developer_message='The provided course key cannot be parsed.', error_code='invalid_course_key' @@ -52,7 +52,8 @@ class GradeViewMixin(DeveloperErrorViewMixin): log.info('Course with ID "%s" not found', course_key_string) except CourseAccessRedirect: log.info('User %s does not have access to course with ID "%s"', user.username, course_key_string) - return self.make_error_response( + + raise self.api_error( status_code=status.HTTP_404_NOT_FOUND, developer_message='The user, the course or both do not exist.', error_code='user_or_course_does_not_exist', @@ -84,7 +85,7 @@ class GradeViewMixin(DeveloperErrorViewMixin): request.user.username, username ) - return self.make_error_response( + raise self.api_error( status_code=status.HTTP_403_FORBIDDEN, developer_message='The user requested does not match the logged in user.', error_code='user_mismatch' @@ -94,7 +95,7 @@ class GradeViewMixin(DeveloperErrorViewMixin): return USER_MODEL.objects.get(username=username) except USER_MODEL.DoesNotExist: - return self.make_error_response( + raise self.api_error( status_code=status.HTTP_404_NOT_FOUND, developer_message='The user matching the requested username does not exist.', error_code='user_does_not_exist' @@ -173,17 +174,9 @@ class UserGradeView(GradeViewMixin, GenericAPIView): """ course = self._get_course(course_id, request.user, 'load') - if isinstance(course, Response): - # Returns a 404 if course_id is invalid, or request.user is not enrolled in the course - return course - grade_user = self._get_effective_user(request, course) - if isinstance(grade_user, Response): - # Returns a 403 if the request.user can't access grades for the requested user, - # or a 404 if the requested user does not exist. - return grade_user - course_grade = CourseGradeFactory().read(grade_user, course) + return Response([{ 'username': grade_user.username, 'course_key': course_id, @@ -221,6 +214,4 @@ class CourseGradingPolicy(GradeViewMixin, ListAPIView): def get(self, request, course_id, **kwargs): course = self._get_course(course_id, request.user, 'staff') - if isinstance(course, Response): - return course return Response(GradingPolicySerializer(course.raw_grader, many=True).data) diff --git a/openedx/core/djangoapps/util/forms.py b/openedx/core/djangoapps/util/forms.py index 6ec4a92d5c..bb60de9354 100644 --- a/openedx/core/djangoapps/util/forms.py +++ b/openedx/core/djangoapps/util/forms.py @@ -65,17 +65,21 @@ class ExtendedNullBooleanField(NullBooleanField): widget = Select(choices=NULL_BOOLEAN_CHOICES) def to_python(self, value): - """ - Explicitly checks for the string 'True', 'False', 'true', - 'false', '1' and '0' and returns boolean True or False. - Returns None if value is not passed at all and raises an - exception for any other value. - """ - if value in (True, 'True', 'true', '1'): - return True - elif value in (False, 'False', 'false', '0'): - return False - elif not value: - return None - else: - raise ValidationError("Invalid Boolean Value.") + return to_bool(value) + + +def to_bool(value): + """ + Explicitly checks for the string 'True', 'False', 'true', + 'false', '1' and '0' and returns boolean True or False. + Returns None if value is not passed at all and raises an + exception for any other value. + """ + if value in (True, 'True', 'true', '1'): + return True + elif value in (False, 'False', 'false', '0'): + return False + elif not value: + return None + else: + raise ValidationError("Invalid Boolean Value.") diff --git a/openedx/core/lib/api/view_utils.py b/openedx/core/lib/api/view_utils.py index ed552ae11c..729d5eb128 100644 --- a/openedx/core/lib/api/view_utils.py +++ b/openedx/core/lib/api/view_utils.py @@ -21,13 +21,30 @@ from openedx.core.lib.api.authentication import ( from openedx.core.lib.api.permissions import IsUserInUrl +class DeveloperErrorResponseException(Exception): + """ + An exception class that wraps a DRF Response object so that + it does not need to be recreated when returning a response. + Intended to be used with and by DeveloperErrorViewMixin. + """ + def __init__(self, response): + super(DeveloperErrorResponseException, self).__init__() + self.response = response + + class DeveloperErrorViewMixin(object): """ A view mixin to handle common error cases other than validation failure (auth failure, method not allowed, etc.) by generating an error response conforming to our API conventions with a developer message. """ - def make_error_response(self, status_code, developer_message, error_code=None): + @classmethod + def api_error(cls, status_code, developer_message, error_code=None): + response = cls._make_error_response(status_code, developer_message, error_code) + return DeveloperErrorResponseException(response) + + @classmethod + def _make_error_response(cls, status_code, developer_message, error_code=None): """ Build an error response with the given status code and developer_message """ @@ -36,7 +53,8 @@ class DeveloperErrorViewMixin(object): error_data['error_code'] = error_code return Response(error_data, status=status_code) - def make_validation_error_response(self, validation_error): + @classmethod + def _make_validation_error_response(cls, validation_error): """ Build a 400 error response from the given ValidationError """ @@ -57,19 +75,20 @@ class DeveloperErrorViewMixin(object): } return Response(response_obj, status=400) else: - return self.make_error_response(400, validation_error.messages[0]) + return cls._make_error_response(400, validation_error.messages[0]) def handle_exception(self, exc): """ Generalized helper method for managing specific API exception workflows """ - - if isinstance(exc, APIException): - return self.make_error_response(exc.status_code, exc.detail) + if isinstance(exc, DeveloperErrorResponseException): + return exc.response + elif isinstance(exc, APIException): + return self._make_error_response(exc.status_code, exc.detail) elif isinstance(exc, Http404) or isinstance(exc, ObjectDoesNotExist): - return self.make_error_response(404, text_type(exc) or "Not found.") + return self._make_error_response(404, text_type(exc) or "Not found.") elif isinstance(exc, ValidationError): - return self.make_validation_error_response(exc) + return self._make_validation_error_response(exc) else: raise