From 2a9383412f513c0709ea6009056582b20d032110 Mon Sep 17 00:00:00 2001 From: Pooja Kulkarni Date: Tue, 18 Jul 2023 11:54:18 -0400 Subject: [PATCH] revert: Removing the details_settings API This reverts commit 8940cb3830c8b629a6e761d47549f2dccb9b13da since it will be replaced with a new commit from upstream. --- .../v0/tests/test_details_settings.py | 74 ----------- .../contentstore/rest_api/v0/urls.py | 13 +- .../rest_api/v0/views/__init__.py | 1 - .../rest_api/v0/views/details_settings.py | 69 ---------- cms/djangoapps/contentstore/views/course.py | 119 +++++++++--------- common/djangoapps/util/json_request.py | 12 +- 6 files changed, 59 insertions(+), 229 deletions(-) delete mode 100644 cms/djangoapps/contentstore/rest_api/v0/tests/test_details_settings.py delete mode 100644 cms/djangoapps/contentstore/rest_api/v0/views/details_settings.py diff --git a/cms/djangoapps/contentstore/rest_api/v0/tests/test_details_settings.py b/cms/djangoapps/contentstore/rest_api/v0/tests/test_details_settings.py deleted file mode 100644 index a6e2d6b19aea..000000000000 --- a/cms/djangoapps/contentstore/rest_api/v0/tests/test_details_settings.py +++ /dev/null @@ -1,74 +0,0 @@ -""" -Tests for the course advanced settings API. -""" -import json - -import ddt -from django.urls import reverse -from rest_framework import status - -from cms.djangoapps.contentstore.tests.utils import CourseTestCase - - -@ddt.ddt -class CourseDetailsSettingViewTest(CourseTestCase): - """ - Tests for DetailsSettings API View. - """ - - def setUp(self): - super().setUp() - self.url = reverse( - "cms.djangoapps.contentstore:v0:course_details_settings", - kwargs={"course_id": self.course.id}, - ) - - 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) - assert error == "Authentication credentials were not provided." - - 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) - assert error == "You do not have permission to perform this action." - - def test_get_course_details(self): - """ - Test for get response - """ - response = self.client.get(self.url) - self.assertEqual(response.status_code, status.HTTP_200_OK) - - def test_patch_course_details(self): - """ - Test for patch response - """ - data = { - "start_date": "2030-01-01T00:00:00Z", - "end_date": "2030-01-31T00:00:00Z", - "enrollment_start": "2029-12-01T00:00:00Z", - "enrollment_end": "2030-01-01T00:00:00Z", - "course_title": "Test Course", - "short_description": "This is a test course", - "overview": "This course is for testing purposes", - "intro_video": None - } - response = self.client.patch(self.url, data, content_type='application/json') - self.assertEqual(response.status_code, status.HTTP_200_OK) diff --git a/cms/djangoapps/contentstore/rest_api/v0/urls.py b/cms/djangoapps/contentstore/rest_api/v0/urls.py index d2db1471868b..eb435ef338d8 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/urls.py +++ b/cms/djangoapps/contentstore/rest_api/v0/urls.py @@ -3,13 +3,7 @@ from django.urls import re_path from openedx.core.constants import COURSE_ID_PATTERN -from .views import ( - AdvancedCourseSettingsView, - CourseDetailsSettingsView, - CourseTabSettingsView, - CourseTabListView, - CourseTabReorderView -) +from .views import AdvancedCourseSettingsView, CourseTabSettingsView, CourseTabListView, CourseTabReorderView app_name = "v0" @@ -34,9 +28,4 @@ CourseTabReorderView.as_view(), name="course_tab_reorder", ), - re_path( - fr"^details_settings/{COURSE_ID_PATTERN}$", - CourseDetailsSettingsView.as_view(), - name="course_details_settings", - ), ] diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/__init__.py b/cms/djangoapps/contentstore/rest_api/v0/views/__init__.py index 1cc7a9ca249f..91f4388cd8db 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/__init__.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/__init__.py @@ -2,5 +2,4 @@ Views for v0 contentstore API. """ from .advanced_settings import AdvancedCourseSettingsView -from .details_settings import CourseDetailsSettingsView from .tabs import CourseTabSettingsView, CourseTabListView, CourseTabReorderView diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/details_settings.py b/cms/djangoapps/contentstore/rest_api/v0/views/details_settings.py deleted file mode 100644 index 3d2392c9d132..000000000000 --- a/cms/djangoapps/contentstore/rest_api/v0/views/details_settings.py +++ /dev/null @@ -1,69 +0,0 @@ -""" API Views for course details settings """ - -import edx_api_doc_tools as apidocs -from opaque_keys.edx.keys import CourseKey -from rest_framework.request import Request -from rest_framework.views import APIView -from xmodule.modulestore.django import modulestore - -from cms.djangoapps.models.settings.encoder import CourseSettingsEncoder -from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access -from common.djangoapps.util.json_request import JsonResponse, expect_json -from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, verify_course_exists, view_auth_classes -from openedx.core.djangoapps.models.course_details import CourseDetails - -from ....views.course import update_course_details_settings - - -@view_auth_classes(is_authenticated=True) -@expect_json -class CourseDetailsSettingsView(DeveloperErrorViewMixin, APIView): - """ - View for getting and setting the details settings for a course. - """ - - @apidocs.schema( - parameters=[ - apidocs.string_parameter("course_id", apidocs.ParameterLocation.PATH, description="Course ID"), - ], - responses={ - 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 all the details settings in a course. - """ - course_key = CourseKey.from_string(course_id) - if not has_studio_read_access(request.user, course_key): - self.permission_denied(request) - course_details = CourseDetails.fetch(course_key) - return JsonResponse( - course_details, - # encoder serializes dates, old locations, and instances - encoder=CourseSettingsEncoder - ) - - @apidocs.schema( - parameters=[ - apidocs.string_parameter("course_id", apidocs.ParameterLocation.PATH, description="Course ID"), - ], - responses={ - 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 patch(self, request: Request, course_id: str): - """ - Update a course's details settings. - """ - course_key = CourseKey.from_string(course_id) - if not has_studio_write_access(request.user, course_key): - self.permission_denied(request) - course_block = modulestore().get_course(course_key) - return update_course_details_settings(course_key, course_block, request) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index be83c164cc02..735adbc0316e 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -1275,70 +1275,63 @@ def settings_handler(request, course_key_string): # lint-amnesty, pylint: disab ) # For every other possible method type submitted by the caller... else: - return update_course_details_settings(course_key, course_block, request) - - -def update_course_details_settings(course_key, course_block: CourseBlock, request): - """ - Helper function to update course details settings from API data - """ - # if pre-requisite course feature is enabled set pre-requisite course - if is_prerequisite_courses_enabled(): - prerequisite_course_keys = request.json.get('pre_requisite_courses', []) - if prerequisite_course_keys: - if not all(is_valid_course_key(course_key) for course_key in prerequisite_course_keys): - return JsonResponseBadRequest({"error": _("Invalid prerequisite course key")}) - set_prerequisite_courses(course_key, prerequisite_course_keys) - else: - # None is chosen, so remove the course prerequisites - course_milestones = milestones_api.get_course_milestones( - course_key=course_key, - relationship="requires", - ) - for milestone in course_milestones: - entrance_exam_namespace = generate_milestone_namespace( - get_namespace_choices().get('ENTRANCE_EXAM'), - course_key + # if pre-requisite course feature is enabled set pre-requisite course + if is_prerequisite_courses_enabled(): + prerequisite_course_keys = request.json.get('pre_requisite_courses', []) + if prerequisite_course_keys: + if not all(is_valid_course_key(course_key) for course_key in prerequisite_course_keys): + return JsonResponseBadRequest({"error": _("Invalid prerequisite course key")}) + set_prerequisite_courses(course_key, prerequisite_course_keys) + else: + # None is chosen, so remove the course prerequisites + course_milestones = milestones_api.get_course_milestones( + course_key=course_key, + relationship="requires", + ) + for milestone in course_milestones: + entrance_exam_namespace = generate_milestone_namespace( + get_namespace_choices().get('ENTRANCE_EXAM'), + course_key + ) + if milestone["namespace"] != entrance_exam_namespace: + remove_prerequisite_course(course_key, milestone) + + # If the entrance exams feature has been enabled, we'll need to check for some + # feature-specific settings and handle them accordingly + # We have to be careful that we're only executing the following logic if we actually + # need to create or delete an entrance exam from the specified course + if core_toggles.ENTRANCE_EXAMS.is_enabled(): + course_entrance_exam_present = course_block.entrance_exam_enabled + entrance_exam_enabled = request.json.get('entrance_exam_enabled', '') == 'true' + ee_min_score_pct = request.json.get('entrance_exam_minimum_score_pct', None) + # If the entrance exam box on the settings screen has been checked... + if entrance_exam_enabled: + # Load the default minimum score threshold from settings, then try to override it + entrance_exam_minimum_score_pct = float(settings.ENTRANCE_EXAM_MIN_SCORE_PCT) + if ee_min_score_pct: + entrance_exam_minimum_score_pct = float(ee_min_score_pct) + if entrance_exam_minimum_score_pct.is_integer(): + entrance_exam_minimum_score_pct = entrance_exam_minimum_score_pct / 100 + # If there's already an entrance exam defined, we'll update the existing one + if course_entrance_exam_present: + exam_data = { + 'entrance_exam_minimum_score_pct': entrance_exam_minimum_score_pct + } + update_entrance_exam(request, course_key, exam_data) + # If there's no entrance exam defined, we'll create a new one + else: + create_entrance_exam(request, course_key, entrance_exam_minimum_score_pct) + + # If the entrance exam box on the settings screen has been unchecked, + # and the course has an entrance exam attached... + elif not entrance_exam_enabled and course_entrance_exam_present: + delete_entrance_exam(request, course_key) + + # Perform the normal update workflow for the CourseDetails model + return JsonResponse( + CourseDetails.update_from_json(course_key, request.json, request.user), + encoder=CourseSettingsEncoder ) - if milestone["namespace"] != entrance_exam_namespace: - remove_prerequisite_course(course_key, milestone) - - # If the entrance exams feature has been enabled, we'll need to check for some - # feature-specific settings and handle them accordingly - # We have to be careful that we're only executing the following logic if we actually - # need to create or delete an entrance exam from the specified course - if core_toggles.ENTRANCE_EXAMS.is_enabled(): - course_entrance_exam_present = course_block.entrance_exam_enabled - entrance_exam_enabled = request.json.get('entrance_exam_enabled', '') == 'true' - ee_min_score_pct = request.json.get('entrance_exam_minimum_score_pct', None) - # If the entrance exam box on the settings screen has been checked... - if entrance_exam_enabled: - # Load the default minimum score threshold from settings, then try to override it - entrance_exam_minimum_score_pct = float(settings.ENTRANCE_EXAM_MIN_SCORE_PCT) - if ee_min_score_pct: - entrance_exam_minimum_score_pct = float(ee_min_score_pct) - if entrance_exam_minimum_score_pct.is_integer(): - entrance_exam_minimum_score_pct = entrance_exam_minimum_score_pct / 100 - # If there's already an entrance exam defined, we'll update the existing one - if course_entrance_exam_present: - exam_data = { - 'entrance_exam_minimum_score_pct': entrance_exam_minimum_score_pct - } - update_entrance_exam(request, course_key, exam_data) - # If there's no entrance exam defined, we'll create a new one - else: - create_entrance_exam(request, course_key, entrance_exam_minimum_score_pct) - - # If the entrance exam box on the settings screen has been unchecked, - # and the course has an entrance exam attached... - elif not entrance_exam_enabled and course_entrance_exam_present: - delete_entrance_exam(request, course_key) - - # Perform the normal update workflow for the CourseDetails model - return JsonResponse( - CourseDetails.update_from_json(course_key, request.json, request.user), - encoder=CourseSettingsEncoder - ) @login_required diff --git a/common/djangoapps/util/json_request.py b/common/djangoapps/util/json_request.py index d71620020771..3c9646b130e5 100644 --- a/common/djangoapps/util/json_request.py +++ b/common/djangoapps/util/json_request.py @@ -8,8 +8,6 @@ from django.core.serializers.json import DjangoJSONEncoder from django.db.models.query import QuerySet from django.http import HttpResponse, HttpResponseBadRequest -from django.utils.decorators import method_decorator -from django.views import View class EDXJSONEncoder(DjangoJSONEncoder): @@ -42,6 +40,7 @@ def expect_json(view_function): CONTENT_TYPE is application/json, parses the json dict from request.body, and updates request.POST with the contents. """ + @wraps(view_function) def parse_json_into_request(request, *args, **kwargs): # cdodge: fix postback errors in CMS. The POST 'content-type' header can include additional information # e.g. 'charset', so we can't do a direct string compare @@ -55,14 +54,7 @@ def parse_json_into_request(request, *args, **kwargs): return view_function(request, *args, **kwargs) - if isinstance(view_function, type) and issubclass(view_function, View): - view_function.dispatch = method_decorator(expect_json)(view_function.dispatch) - return view_function - else: - @wraps(view_function) - def wrapper(request, *args, **kwargs): - return parse_json_into_request(request, *args, **kwargs) - return wrapper + return parse_json_into_request class JsonResponse(HttpResponse):