From 863fdfe2c82f2f81abdadd5cffb49f8ea7320a43 Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Mon, 10 Jul 2023 19:22:43 +0530 Subject: [PATCH] feat: Create DRF for course grading (#32399) (#550) (cherry picked from commit dea67f29c41e588c297973a96c522af9a25d653d) Co-authored-by: ruzniaievdm --- .../contentstore/rest_api/v1/mixins.py | 42 +++++ .../contentstore/rest_api/v1/serializers.py | 35 ++++ .../rest_api/v1/tests/test_grading.py | 108 +++++++++++ .../contentstore/rest_api/v1/urls.py | 5 + .../contentstore/rest_api/v1/views.py | 171 +++++++++++++++++- cms/djangoapps/contentstore/utils.py | 23 +++ cms/djangoapps/contentstore/views/course.py | 16 +- 7 files changed, 387 insertions(+), 13 deletions(-) create mode 100644 cms/djangoapps/contentstore/rest_api/v1/mixins.py create mode 100644 cms/djangoapps/contentstore/rest_api/v1/tests/test_grading.py diff --git a/cms/djangoapps/contentstore/rest_api/v1/mixins.py b/cms/djangoapps/contentstore/rest_api/v1/mixins.py new file mode 100644 index 000000000000..849a4834905e --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/mixins.py @@ -0,0 +1,42 @@ +""" +Common mixins for module. +""" +import json +from unittest.mock import patch + +from rest_framework import status + + +class PermissionAccessMixin: + """ + Mixin for testing permission access for views. + """ + + def get_and_check_developer_response(self, response): + """ + Make basic asserting about the presence of an error response, and return the developer response. + """ + content = json.loads(response.content.decode("utf-8")) + assert "developer_message" in content + return content["developer_message"] + + def test_permissions_unauthenticated(self): + """ + Test that an error is returned in the absence of auth credentials. + """ + self.client.logout() + response = self.client.get(self.url) + error = self.get_and_check_developer_response(response) + self.assertEqual(error, "Authentication credentials were not provided.") + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + @patch.dict('django.conf.settings.FEATURES', {'DISABLE_ADVANCED_SETTINGS': True}) + def test_permissions_unauthorized(self): + """ + Test that an error is returned if the user is unauthorised. + """ + client, _ = self.create_non_staff_authed_user_client() + response = client.get(self.url) + error = self.get_and_check_developer_response(response) + self.assertEqual(error, "You do not have permission to perform this action.") + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers.py b/cms/djangoapps/contentstore/rest_api/v1/serializers.py index 2a5f92325f31..38b614ff976b 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers.py @@ -29,3 +29,38 @@ class ProctoredExamConfigurationSerializer(serializers.Serializer): proctored_exam_settings = ProctoredExamSettingsSerializer() available_proctoring_providers = serializers.ChoiceField(get_available_providers()) course_start_date = serializers.DateTimeField() + + +class GradersSerializer(serializers.Serializer): + """ Serializer for graders """ + type = serializers.CharField() + min_count = serializers.IntegerField() + drop_count = serializers.IntegerField() + short_label = serializers.CharField(required=False, allow_null=True, allow_blank=True) + weight = serializers.IntegerField() + id = serializers.IntegerField() + + +class GracePeriodSerializer(serializers.Serializer): + """ Serializer for course grace period """ + hours = serializers.IntegerField() + minutes = serializers.IntegerField() + + +class CourseGradingModelSerializer(serializers.Serializer): + """ Serializer for course grading model data """ + graders = GradersSerializer(many=True) + grade_cutoffs = serializers.DictField(child=serializers.FloatField()) + grace_period = GracePeriodSerializer(required=False, allow_null=True) + minimum_grade_credit = serializers.FloatField() + + +class CourseGradingSerializer(serializers.Serializer): + """ Serializer for course grading context data """ + mfe_proctored_exam_settings_url = serializers.CharField(required=False, allow_null=True, allow_blank=True) + course_details = CourseGradingModelSerializer() + show_credit_eligibility = serializers.BooleanField() + is_credit_course = serializers.BooleanField() + default_grade_designations = serializers.ListSerializer( + child=serializers.CharField() + ) diff --git a/cms/djangoapps/contentstore/rest_api/v1/tests/test_grading.py b/cms/djangoapps/contentstore/rest_api/v1/tests/test_grading.py new file mode 100644 index 000000000000..77b018b25e06 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/tests/test_grading.py @@ -0,0 +1,108 @@ +""" +Unit tests for course grading views. +""" +import json +from unittest.mock import patch + +import ddt +from django.urls import reverse +from rest_framework import status + +from cms.djangoapps.contentstore.tests.utils import CourseTestCase +from cms.djangoapps.contentstore.utils import get_proctored_exam_settings_url +from cms.djangoapps.models.settings.course_grading import CourseGradingModel +from openedx.core.djangoapps.credit.tests.factories import CreditCourseFactory + +from ..mixins import PermissionAccessMixin + + +@ddt.ddt +class CourseGradingViewTest(CourseTestCase, PermissionAccessMixin): + """ + Tests for CourseGradingView. + """ + + def setUp(self): + super().setUp() + self.url = reverse( + 'cms.djangoapps.contentstore:v1:course_grading', + kwargs={"course_id": self.course.id}, + ) + + def test_course_grading_response(self): + """ Check successful response content """ + response = self.client.get(self.url) + grading_data = CourseGradingModel.fetch(self.course.id) + + expected_response = { + 'mfe_proctored_exam_settings_url': get_proctored_exam_settings_url(self.course.id), + 'course_details': grading_data.__dict__, + 'show_credit_eligibility': False, + 'is_credit_course': False, + 'default_grade_designations': ["A", "B", "C", "D"] + } + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertDictEqual(expected_response, response.data) + + @patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREDIT_ELIGIBILITY': True}) + def test_credit_eligibility_setting(self): + """ + Make sure if the feature flag is enabled we have enabled values in response. + """ + _ = CreditCourseFactory(course_key=self.course.id, enabled=True) + response = self.client.get(self.url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertTrue(response.data['show_credit_eligibility']) + self.assertTrue(response.data['is_credit_course']) + + def test_post_permissions_unauthenticated(self): + """ + Test that an error is returned in the absence of auth credentials. + """ + self.client.logout() + response = self.client.post(self.url) + error = self.get_and_check_developer_response(response) + self.assertEqual(error, "Authentication credentials were not provided.") + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_post_permissions_unauthorized(self): + """ + Test that an error is returned if the user is unauthorised. + """ + client, _ = self.create_non_staff_authed_user_client() + response = client.post(self.url) + error = self.get_and_check_developer_response(response) + self.assertEqual(error, "You do not have permission to perform this action.") + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + @patch('openedx.core.djangoapps.credit.tasks.update_credit_course_requirements.delay') + def test_post_course_grading(self, mock_update_credit_course_requirements): + """ Check successful request with called task """ + request_data = { + "graders": [ + { + "type": "Homework", + "min_count": 1, + "drop_count": 0, + "short_label": "", + "weight": 100, + "id": 0 + } + ], + "grade_cutoffs": { + "A": 0.75, + "B": 0.63, + "C": 0.57, + "D": 0.5 + }, + "grace_period": { + "hours": 12, + "minutes": 0 + }, + "minimum_grade_credit": 0.7, + "is_credit_course": True + } + response = self.client.post(path=self.url, data=json.dumps(request_data), content_type="application/json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + mock_update_credit_course_requirements.assert_called_once() diff --git a/cms/djangoapps/contentstore/rest_api/v1/urls.py b/cms/djangoapps/contentstore/rest_api/v1/urls.py index 83e03bcb73a3..23422cfc393e 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/urls.py +++ b/cms/djangoapps/contentstore/rest_api/v1/urls.py @@ -14,4 +14,9 @@ views.ProctoredExamSettingsView.as_view(), name="proctored_exam_settings" ), + re_path( + fr'^course_grading/{COURSE_ID_PATTERN}$', + views.CourseGradingView.as_view(), + name="course_grading" + ), ] diff --git a/cms/djangoapps/contentstore/rest_api/v1/views.py b/cms/djangoapps/contentstore/rest_api/v1/views.py index 9675665d2ec7..9767237c7993 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views.py @@ -1,25 +1,36 @@ "Contentstore Views" import copy +import edx_api_doc_tools as apidocs +from django.conf import settings from opaque_keys.edx.keys import CourseKey from rest_framework import status from rest_framework.exceptions import NotFound +from rest_framework.request import Request from rest_framework.response import Response from rest_framework.views import APIView from cms.djangoapps.contentstore.views.course import get_course_and_check_access +from cms.djangoapps.models.settings.course_grading import CourseGradingModel from cms.djangoapps.models.settings.course_metadata import CourseMetadata +from common.djangoapps.student.auth import has_studio_read_access from xmodule.course_block import get_available_providers # lint-amnesty, pylint: disable=wrong-import-order from openedx.core.djangoapps.course_apps.toggles import exams_ida_enabled -from openedx.core.lib.api.view_utils import view_auth_classes +from openedx.core.djangoapps.credit.api import is_credit_course +from openedx.core.djangoapps.credit.tasks import update_credit_course_requirements +from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, verify_course_exists, view_auth_classes from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from .serializers import ( LimitedProctoredExamSettingsSerializer, ProctoredExamConfigurationSerializer, - ProctoredExamSettingsSerializer + ProctoredExamSettingsSerializer, + CourseGradingModelSerializer, + CourseGradingSerializer ) +from ...utils import get_course_grading + @view_auth_classes() class ProctoredExamSettingsView(APIView): @@ -182,3 +193,159 @@ def _get_and_validate_course_access(user, course_id): ) return course_block + + +@view_auth_classes(is_authenticated=True) +class CourseGradingView(DeveloperErrorViewMixin, APIView): + """ + View for Course Grading policy configuration. + """ + + @apidocs.schema( + parameters=[ + apidocs.string_parameter("course_id", apidocs.ParameterLocation.PATH, description="Course ID"), + ], + responses={ + 200: CourseGradingSerializer, + 401: "The requester is not authenticated.", + 403: "The requester cannot access the specified course.", + 404: "The requested course does not exist.", + }, + ) + @verify_course_exists() + def get(self, request: Request, course_id: str): + """ + Get an object containing course grading settings with model. + + **Example Request** + + GET /api/contentstore/v1/course_grading/{course_id} + + **Response Values** + + If the request is successful, an HTTP 200 "OK" response is returned. + + The HTTP 200 response contains a single dict that contains keys that + are the course's grading. + + **Example Response** + + ```json + { + "mfe_proctored_exam_settings_url": "", + "course_assignment_lists": { + "Homework": [ + "Section :754c5e889ac3489e9947ba62b916bdab - Subsection :56c1bc20d270414b877e9c178954b6ed" + ] + }, + "course_details": { + "graders": [ + { + "type": "Homework", + "min_count": 1, + "drop_count": 0, + "short_label": "", + "weight": 100, + "id": 0 + } + ], + "grade_cutoffs": { + "A": 0.75, + "B": 0.63, + "C": 0.57, + "D": 0.5 + }, + "grace_period": { + "hours": 12, + "minutes": 0 + }, + "minimum_grade_credit": 0.7 + }, + "show_credit_eligibility": false, + "is_credit_course": true + } + ``` + """ + course_key = CourseKey.from_string(course_id) + + if not has_studio_read_access(request.user, course_key): + self.permission_denied(request) + + with modulestore().bulk_operations(course_key): + credit_eligibility_enabled = settings.FEATURES.get("ENABLE_CREDIT_ELIGIBILITY", False) + show_credit_eligibility = is_credit_course(course_key) and credit_eligibility_enabled + + grading_context = get_course_grading(course_key) + grading_context['show_credit_eligibility'] = show_credit_eligibility + + serializer = CourseGradingSerializer(grading_context) + return Response(serializer.data) + + @apidocs.schema( + body=CourseGradingModelSerializer, + parameters=[ + apidocs.string_parameter("course_id", apidocs.ParameterLocation.PATH, description="Course ID"), + ], + responses={ + 200: CourseGradingModelSerializer, + 401: "The requester is not authenticated.", + 403: "The requester cannot access the specified course.", + 404: "The requested course does not exist.", + }, + ) + @verify_course_exists() + def post(self, request: Request, course_id: str): + """ + Update a course's grading. + + **Example Request** + + PUT /api/contentstore/v1/course_grading/{course_id} + + **POST Parameters** + + The data sent for a post request should follow next object. + Here is an example request data that updates the ``course_grading`` + + ```json + { + "graders": [ + { + "type": "Homework", + "min_count": 1, + "drop_count": 0, + "short_label": "", + "weight": 100, + "id": 0 + } + ], + "grade_cutoffs": { + "A": 0.75, + "B": 0.63, + "C": 0.57, + "D": 0.5 + }, + "grace_period": { + "hours": 12, + "minutes": 0 + }, + "minimum_grade_credit": 0.7, + "is_credit_course": true + } + ``` + + **Response Values** + + If the request is successful, an HTTP 200 "OK" response is returned, + """ + course_key = CourseKey.from_string(course_id) + + if not has_studio_read_access(request.user, course_key): + self.permission_denied(request) + + if 'minimum_grade_credit' in request.data: + update_credit_course_requirements.delay(str(course_key)) + + updated_data = CourseGradingModel.update_from_json(course_key, request.data, request.user) + serializer = CourseGradingModelSerializer(updated_data) + return Response(serializer.data) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 7791bb681238..140b170e9e2b 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -19,6 +19,7 @@ from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole from openedx.core.djangoapps.course_apps.toggles import proctoring_settings_modal_view_enabled +from openedx.core.djangoapps.credit.api import is_credit_course from openedx.core.djangoapps.discussions.config.waffle import ENABLE_PAGES_AND_RESOURCES_MICROFRONTEND from openedx.core.djangoapps.django_comment_common.models import assign_default_role from openedx.core.djangoapps.django_comment_common.utils import seed_permissions_roles @@ -27,6 +28,7 @@ from openedx.features.content_type_gating.models import ContentTypeGatingConfig from openedx.features.content_type_gating.partitions import CONTENT_TYPE_GATING_SCHEME from cms.djangoapps.contentstore.toggles import use_new_text_editor, use_new_video_editor +from cms.djangoapps.models.settings.course_grading import CourseGradingModel from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order @@ -731,3 +733,24 @@ def translation_language(language): translation.activate(previous) else: yield + + +def get_course_grading(course_key): + """ + Utils is used to get context of course grading. + It is used for both DRF and django views. + """ + + course_block = modulestore().get_course(course_key) + course_details = CourseGradingModel.fetch(course_key) + grading_context = { + 'context_course': course_block, + 'course_locator': course_key, + 'course_details': course_details, + 'grading_url': reverse_course_url('grading_handler', course_key), + 'is_credit_course': is_credit_course(course_key), + 'mfe_proctored_exam_settings_url': get_proctored_exam_settings_url(course_key), + 'default_grade_designations': settings.DEFAULT_GRADE_DESIGNATIONS + } + + return grading_context diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 17ce3eea18b1..b583a954c6ef 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -104,6 +104,7 @@ from ..toggles import split_library_view_on_dashboard from ..utils import ( add_instructor, + get_course_grading, get_lms_link_for_item, get_proctored_exam_settings_url, initialize_permissions, @@ -1354,19 +1355,12 @@ def grading_handler(request, course_key_string, grader_index=None): """ course_key = CourseKey.from_string(course_key_string) with modulestore().bulk_operations(course_key): - course_block = get_course_and_check_access(course_key, request.user) + if not has_studio_read_access(request.user, course_key): + raise PermissionDenied() if 'text/html' in request.META.get('HTTP_ACCEPT', '') and request.method == 'GET': - course_details = CourseGradingModel.fetch(course_key) - return render_to_response('settings_graders.html', { - 'context_course': course_block, - 'course_locator': course_key, - 'course_details': course_details, - 'grading_url': reverse_course_url('grading_handler', course_key), - 'is_credit_course': is_credit_course(course_key), - 'mfe_proctored_exam_settings_url': get_proctored_exam_settings_url(course_block.id), - 'default_grade_designations': settings.DEFAULT_GRADE_DESIGNATIONS - }) + grading_context = get_course_grading(course_key) + return render_to_response('settings_graders.html', grading_context) elif 'application/json' in request.META.get('HTTP_ACCEPT', ''): if request.method == 'GET': if grader_index is None: