From dea41472324329669231af10329fc42facc0f893 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Mon, 3 Oct 2016 22:34:41 -0400 Subject: [PATCH] Grading Policy endpoint in Grades API This copies the grading policy endpoint currently in the course_structures django app to its new location, in preparation to remove the old app. TNL-5701 --- lms/djangoapps/grades/api/serializers.py | 29 ++ lms/djangoapps/grades/api/tests/test_views.py | 249 +++++++++++++++++- lms/djangoapps/grades/api/urls.py | 8 +- lms/djangoapps/grades/api/views.py | 122 ++++++--- 4 files changed, 373 insertions(+), 35 deletions(-) create mode 100644 lms/djangoapps/grades/api/serializers.py diff --git a/lms/djangoapps/grades/api/serializers.py b/lms/djangoapps/grades/api/serializers.py new file mode 100644 index 0000000000..be50412fce --- /dev/null +++ b/lms/djangoapps/grades/api/serializers.py @@ -0,0 +1,29 @@ +""" +API Serializers +""" +from collections import defaultdict +from rest_framework import serializers + + +# pylint: disable=abstract-method +class GradingPolicySerializer(serializers.Serializer): + """ + Serializer for course grading policy. + """ + assignment_type = serializers.CharField(source='type') + count = serializers.IntegerField(source='min_count') + dropped = serializers.IntegerField(source='drop_count') + weight = serializers.FloatField() + + def to_representation(self, obj): + """ + Return a representation of the grading policy. + """ + # Backwards compatibility with the behavior of DRF v2. + # When the grader dictionary was missing keys, DRF v2 would default to None; + # DRF v3 unhelpfully raises an exception. + return dict( + super(GradingPolicySerializer, self).to_representation( + defaultdict(lambda: None, obj) + ) + ) diff --git a/lms/djangoapps/grades/api/tests/test_views.py b/lms/djangoapps/grades/api/tests/test_views.py index 4ba173371c..53c2bb6f29 100644 --- a/lms/djangoapps/grades/api/tests/test_views.py +++ b/lms/djangoapps/grades/api/tests/test_views.py @@ -2,7 +2,6 @@ Tests for the views """ from datetime import datetime - import ddt from django.core.urlresolvers import reverse from mock import patch @@ -11,8 +10,12 @@ from pytz import UTC from rest_framework import status from rest_framework.test import APITestCase +from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory +from edx_oauth2_provider.tests.factories import AccessTokenFactory, ClientFactory +from lms.djangoapps.courseware.tests.factories import GlobalStaffFactory, StaffFactory from lms.djangoapps.grades.tests.utils import mock_get_score from student.tests.factories import CourseEnrollmentFactory, UserFactory +from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase, TEST_DATA_SPLIT_MODULESTORE @@ -184,8 +187,10 @@ class CurrentGradeViewTest(SharedModuleStoreTestCase, APITestCase): def mock_from_string(*args, **kwargs): # pylint: disable=unused-argument """Mocked function to always raise an exception""" raise InvalidKeyError('foo', 'bar') + with patch('opaque_keys.edx.keys.CourseKey.from_string', side_effect=mock_from_string): resp = self.client.get(self.get_url(self.student.username)) + self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND) self.assertIn('error_code', resp.data) # pylint: disable=no-member self.assertEqual( @@ -245,3 +250,245 @@ class CurrentGradeViewTest(SharedModuleStoreTestCase, APITestCase): } expected_data.update(result) self.assertEqual(resp.data, [expected_data]) # pylint: disable=no-member + + +@ddt.ddt +class GradingPolicyTestMixin(object): + """ + Mixin class for Grading Policy tests + """ + view_name = None + + def setUp(self): + super(GradingPolicyTestMixin, self).setUp() + self.create_user_and_access_token() + + def create_user_and_access_token(self): + # pylint: disable=missing-docstring + self.user = GlobalStaffFactory.create() + self.oauth_client = ClientFactory.create() + self.access_token = AccessTokenFactory.create(user=self.user, client=self.oauth_client).token + + @classmethod + def create_course_data(cls): + # pylint: disable=missing-docstring + cls.invalid_course_id = 'foo/bar/baz' + cls.course = CourseFactory.create(display_name='An Introduction to API Testing', raw_grader=cls.raw_grader) + cls.course_id = unicode(cls.course.id) + with cls.store.bulk_operations(cls.course.id, emit_signals=False): + cls.sequential = ItemFactory.create( + category="sequential", + parent_location=cls.course.location, + display_name="Lesson 1", + format="Homework", + graded=True + ) + + factory = MultipleChoiceResponseXMLFactory() + args = {'choices': [False, True, False]} + problem_xml = factory.build_xml(**args) + cls.problem = ItemFactory.create( + category="problem", + parent_location=cls.sequential.location, + display_name="Problem 1", + format="Homework", + data=problem_xml, + ) + + cls.video = ItemFactory.create( + category="video", + parent_location=cls.sequential.location, + display_name="Video 1", + ) + + cls.html = ItemFactory.create( + category="html", + parent_location=cls.sequential.location, + display_name="HTML 1", + ) + + def http_get(self, uri, **headers): + """ + Submit an HTTP GET request + """ + + default_headers = { + 'HTTP_AUTHORIZATION': 'Bearer ' + self.access_token + } + default_headers.update(headers) + + response = self.client.get(uri, follow=True, **default_headers) + return response + + def assert_get_for_course(self, course_id=None, expected_status_code=200, **headers): + """ + Submit an HTTP GET request to the view for the given course. + Validates the status_code of the response is as expected. + """ + + response = self.http_get( + reverse(self.view_name, kwargs={'course_id': course_id or self.course_id}), + **headers + ) + self.assertEqual(response.status_code, expected_status_code) + return response + + def get_auth_header(self, user): + """ + Returns Bearer auth header with a generated access token + for the given user. + """ + access_token = AccessTokenFactory.create(user=user, client=self.oauth_client).token + return 'Bearer ' + access_token + + def test_get_invalid_course(self): + """ + The view should return a 404 for an invalid course ID. + """ + self.assert_get_for_course(course_id=self.invalid_course_id, expected_status_code=404) + + def test_get(self): + """ + The view should return a 200 for a valid course ID. + """ + return self.assert_get_for_course() + + def test_not_authenticated(self): + """ + The view should return HTTP status 401 if user is unauthenticated. + """ + self.assert_get_for_course(expected_status_code=401, HTTP_AUTHORIZATION=None) + + def test_staff_authorized(self): + """ + The view should return a 200 when provided an access token + for course staff. + """ + user = StaffFactory(course_key=self.course.id) + auth_header = self.get_auth_header(user) + self.assert_get_for_course(HTTP_AUTHORIZATION=auth_header) + + def test_not_authorized(self): + """ + The view should return HTTP status 404 when provided an + access token for an unauthorized user. + """ + user = UserFactory() + auth_header = self.get_auth_header(user) + self.assert_get_for_course(expected_status_code=404, HTTP_AUTHORIZATION=auth_header) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_course_keys(self, modulestore_type): + """ + The view should be addressable by course-keys from both module stores. + """ + course = CourseFactory.create( + start=datetime(2014, 6, 16, 14, 30), + end=datetime(2015, 1, 16), + org="MTD", + default_store=modulestore_type, + ) + self.assert_get_for_course(course_id=unicode(course.id)) + + +class CourseGradingPolicyTests(GradingPolicyTestMixin, SharedModuleStoreTestCase): + """ + Tests for CourseGradingPolicy view. + """ + view_name = 'grades_api:course_grading_policy' + + raw_grader = [ + { + "min_count": 24, + "weight": 0.2, + "type": "Homework", + "drop_count": 0, + "short_label": "HW" + }, + { + "min_count": 4, + "weight": 0.8, + "type": "Exam", + "drop_count": 0, + "short_label": "Exam" + } + ] + + @classmethod + def setUpClass(cls): + super(CourseGradingPolicyTests, cls).setUpClass() + cls.create_course_data() + + def test_get(self): + """ + The view should return grading policy for a course. + """ + response = super(CourseGradingPolicyTests, self).test_get() + + expected = [ + { + "count": 24, + "weight": 0.2, + "assignment_type": "Homework", + "dropped": 0 + }, + { + "count": 4, + "weight": 0.8, + "assignment_type": "Exam", + "dropped": 0 + } + ] + self.assertListEqual(response.data, expected) + + +class CourseGradingPolicyMissingFieldsTests(GradingPolicyTestMixin, SharedModuleStoreTestCase): + """ + Tests for CourseGradingPolicy view when fields are missing. + """ + view_name = 'grades_api:course_grading_policy' + + # Raw grader with missing keys + raw_grader = [ + { + "min_count": 24, + "weight": 0.2, + "type": "Homework", + "drop_count": 0, + "short_label": "HW" + }, + { + # Deleted "min_count" key + "weight": 0.8, + "type": "Exam", + "drop_count": 0, + "short_label": "Exam" + } + ] + + @classmethod + def setUpClass(cls): + super(CourseGradingPolicyMissingFieldsTests, cls).setUpClass() + cls.create_course_data() + + def test_get(self): + """ + The view should return grading policy for a course. + """ + response = super(CourseGradingPolicyMissingFieldsTests, self).test_get() + + expected = [ + { + "count": 24, + "weight": 0.2, + "assignment_type": "Homework", + "dropped": 0 + }, + { + "count": None, + "weight": 0.8, + "assignment_type": "Exam", + "dropped": 0 + } + ] + self.assertListEqual(response.data, expected) diff --git a/lms/djangoapps/grades/api/urls.py b/lms/djangoapps/grades/api/urls.py index f617160923..1ffb446af6 100644 --- a/lms/djangoapps/grades/api/urls.py +++ b/lms/djangoapps/grades/api/urls.py @@ -11,8 +11,14 @@ urlpatterns = patterns( '', url( r'^v0/course_grade/{course_id}/users/$'.format( - course_id=settings.COURSE_ID_PATTERN + course_id=settings.COURSE_ID_PATTERN, ), views.UserGradeView.as_view(), name='user_grade_detail' ), + url( + r'^v0/courses/{course_id}/policy/$'.format( + course_id=settings.COURSE_ID_PATTERN, + ), + views.CourseGradingPolicy.as_view(), name='course_grading_policy' + ), ) diff --git a/lms/djangoapps/grades/api/views.py b/lms/djangoapps/grades/api/views.py index 92198b95c0..b46f02ccc5 100644 --- a/lms/djangoapps/grades/api/views.py +++ b/lms/djangoapps/grades/api/views.py @@ -6,12 +6,14 @@ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from rest_framework import status from rest_framework.authentication import SessionAuthentication -from rest_framework.generics import GenericAPIView +from rest_framework.exceptions import AuthenticationFailed +from rest_framework.generics import GenericAPIView, ListAPIView from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from lms.djangoapps.ccx.utils import prep_course_for_grading from lms.djangoapps.courseware import courses +from lms.djangoapps.grades.api.serializers import GradingPolicySerializer from lms.djangoapps.grades.new.course_grade import CourseGradeFactory from openedx.core.lib.api.authentication import OAuth2AuthenticationAllowInactiveUser from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin @@ -19,7 +21,55 @@ from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin log = logging.getLogger(__name__) -class UserGradeView(DeveloperErrorViewMixin, GenericAPIView): +class GradeViewMixin(DeveloperErrorViewMixin): + """ + Mixin class for Grades related views. + """ + authentication_classes = ( + OAuth2AuthenticationAllowInactiveUser, + SessionAuthentication, + ) + permission_classes = (IsAuthenticated,) + + def _get_course(self, course_key_string, user, access_action): + """ + Returns the course for the given course_key_string after + verifying the requested access to the course by the given user. + """ + try: + course_key = CourseKey.from_string(course_key_string) + except InvalidKeyError: + return self.make_error_response( + status_code=status.HTTP_404_NOT_FOUND, + developer_message='The provided course key cannot be parsed.', + error_code='invalid_course_key' + ) + + try: + return courses.get_course_with_access( + user, + access_action, + course_key, + check_if_enrolled=True + ) + except Http404: + log.info('Course with ID "%s" not found', course_key_string) + return self.make_error_response( + 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' + ) + + def perform_authentication(self, request): + """ + Ensures that the user is authenticated (e.g. not an AnonymousUser), unless DEBUG mode is enabled. + """ + super(GradeViewMixin, self).perform_authentication(request) + if request.user.is_anonymous(): + raise AuthenticationFailed + + +class UserGradeView(GradeViewMixin, GenericAPIView): """ **Use Case** @@ -67,12 +117,6 @@ class UserGradeView(DeveloperErrorViewMixin, GenericAPIView): }] """ - authentication_classes = ( - OAuth2AuthenticationAllowInactiveUser, - SessionAuthentication, - ) - permission_classes = (IsAuthenticated, ) - def get(self, request, course_id): """ Gets a course progress status. @@ -99,31 +143,10 @@ class UserGradeView(DeveloperErrorViewMixin, GenericAPIView): error_code='user_mismatch' ) - # build the course key - try: - course_key = CourseKey.from_string(course_id) - except InvalidKeyError: - return self.make_error_response( - status_code=status.HTTP_404_NOT_FOUND, - developer_message='The provided course key cannot be parsed.', - error_code='invalid_course_key' - ) - # load the course - try: - course = courses.get_course_with_access( - request.user, - 'load', - course_key, - depth=None, - check_if_enrolled=True - ) - except Http404: - log.info('Course with ID "%s" not found', course_id) - return self.make_error_response( - 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' - ) + course = self._get_course(course_id, request.user, 'load') + if isinstance(course, Response): + return course + prep_course_for_grading(course, request) course_grade = CourseGradeFactory(request.user).create(course) if not course_grade.has_access_to_course: @@ -142,3 +165,36 @@ class UserGradeView(DeveloperErrorViewMixin, GenericAPIView): 'percent': course_grade.percent, 'letter_grade': course_grade.letter_grade, }]) + + +class CourseGradingPolicy(GradeViewMixin, ListAPIView): + """ + **Use Case** + + Get the course grading policy. + + **Example requests**: + + GET /api/grades/v0/policy/{course_id}/ + + **Response Values** + + * assignment_type: The type of the assignment, as configured by course + staff. For example, course staff might make the assignment types Homework, + Quiz, and Exam. + + * count: The number of assignments of the type. + + * dropped: Number of assignments of the type that are dropped. + + * weight: The weight, or effect, of the assignment type on the learner's + final grade. + """ + + allow_empty = False + + 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)