Merge pull request #543 from open-craft/kshitij/palm-release

OpenCraft Palm.1 Release Branch

cms/__init__.py

@@ -6,6 +6,12 @@
 isort:skip_file
 """
 
+# FAL-2248: Monkey patch django's get_storage_engine to work around long migrations times.
+# This fixes a performance issue with database migrations in Ocim. We will need to keep
+# this patch in our opencraft-release/* branches until edx-platform upgrades to Django 4.*
+# which will include this commit:
+# https://github.com/django/django/commit/518ce7a51f994fc0585d31c4553e2072bf816f76
+import django.db.backends.mysql.introspection
 
 # We monkey patch Kombu's entrypoints listing because scanning through this
 # accounts for the majority of LMS/Studio startup time for tests, and we don't
@@ -22,3 +28,23 @@
 # that shared_task will use this app, and also ensures that the celery
 # singleton is always configured for the CMS.
 from .celery import APP as CELERY_APP  # lint-amnesty, pylint: disable=wrong-import-position
+
+
+def get_storage_engine(self, cursor, table_name):
+    """
+    This is a patched version of `get_storage_engine` that fixes a
+    performance issue with migrations. For more info see FAL-2248 and
+    https://github.com/django/django/pull/14766
+    """
+    cursor.execute("""
+        SELECT engine
+        FROM information_schema.tables
+        WHERE table_name = %s
+            AND table_schema = DATABASE()""", [table_name])
+    result = cursor.fetchone()
+    if not result:
+        return self.connection.features._mysql_storage_engine  # pylint: disable=protected-access
+    return result[0]
+
+
+django.db.backends.mysql.introspection.DatabaseIntrospection.get_storage_engine = get_storage_engine

cms/djangoapps/api/v1/serializers/course_runs.py

@@ -5,6 +5,7 @@
 from django.db import transaction
 from django.utils.translation import gettext_lazy as _
 from opaque_keys import InvalidKeyError
+from opaque_keys.edx.keys import CourseKey
 from rest_framework import serializers
 from rest_framework.fields import empty
 
@@ -203,3 +204,35 @@ def update(self, instance, validated_data):
         course_run = get_course_and_check_access(new_course_run_key, user)
         self.update_team(course_run, team)
         return course_run
+
+
+class CourseCloneSerializer(serializers.Serializer):  # lint-amnesty, pylint: disable=abstract-method, missing-class-docstring
+    source_course_id = serializers.CharField()
+    destination_course_id = serializers.CharField()
+
+    def validate(self, attrs):
+        source_course_id = attrs.get('source_course_id')
+        destination_course_id = attrs.get('destination_course_id')
+        store = modulestore()
+        source_key = CourseKey.from_string(source_course_id)
+        dest_key = CourseKey.from_string(destination_course_id)
+
+        # Check if the source course exists
+        if not store.has_course(source_key):
+            raise serializers.ValidationError('Source course does not exist.')
+
+        # Check if the destination course already exists
+        if store.has_course(dest_key):
+            raise serializers.ValidationError('Destination course already exists.')
+        return attrs
+
+    def create(self, validated_data):
+        source_course_id = validated_data.get('source_course_id')
+        destination_course_id = validated_data.get('destination_course_id')
+        user_id = self.context['request'].user.id
+        store = modulestore()
+        source_key = CourseKey.from_string(source_course_id)
+        dest_key = CourseKey.from_string(destination_course_id)
+        with store.default_store('split'):
+            new_course = store.clone_course(source_key, dest_key, user_id)
+        return new_course

cms/djangoapps/api/v1/tests/test_views/test_course_runs.py

@@ -402,3 +402,54 @@ def test_rerun_invalid_number(self):
         assert response.data == {'non_field_errors': [
             'Invalid key supplied. Ensure there are no special characters in the Course Number.'
         ]}
+
+    def test_clone_course(self):
+        course = CourseFactory()
+        url = reverse('api:v1:course_run-clone')
+        data = {
+            'source_course_id': str(course.id),
+            'destination_course_id': 'course-v1:destination+course+id',
+        }
+        response = self.client.post(url, data, format='json')
+        assert response.status_code == 201
+        self.assertEqual(response.data, {"message": "Course cloned successfully."})
+
+    def test_clone_course_with_missing_source_id(self):
+        url = reverse('api:v1:course_run-clone')
+        data = {
+            'destination_course_id': 'course-v1:destination+course+id',
+        }
+        response = self.client.post(url, data, format='json')
+        assert response.status_code == 400
+        self.assertEqual(response.data, {'source_course_id': ['This field is required.']})
+
+    def test_clone_course_with_missing_dest_id(self):
+        url = reverse('api:v1:course_run-clone')
+        data = {
+            'source_course_id': 'course-v1:source+course+id',
+        }
+        response = self.client.post(url, data, format='json')
+        assert response.status_code == 400
+        self.assertEqual(response.data, {'destination_course_id': ['This field is required.']})
+
+    def test_clone_course_with_nonexistent_source_course(self):
+        url = reverse('api:v1:course_run-clone')
+        data = {
+            'source_course_id': 'course-v1:nonexistent+source+course_id',
+            'destination_course_id': 'course-v1:destination+course+id',
+        }
+        response = self.client.post(url, data, format='json')
+        assert response.status_code == 400
+        assert str(response.data.get('non_field_errors')[0]) == 'Source course does not exist.'
+
+    def test_clone_course_with_existing_dest_course(self):
+        url = reverse('api:v1:course_run-clone')
+        course = CourseFactory()
+        existing_dest_course = CourseFactory()
+        data = {
+            'source_course_id': str(course.id),
+            'destination_course_id': str(existing_dest_course.id),
+        }
+        response = self.client.post(url, data, format='json')
+        assert response.status_code == 400
+        assert str(response.data.get('non_field_errors')[0]) == 'Destination course already exists.'

cms/djangoapps/api/v1/views/course_runs.py

@@ -13,6 +13,7 @@
 from cms.djangoapps.contentstore.views.course import _accessible_courses_iter, get_course_and_check_access
 
 from ..serializers.course_runs import (
+    CourseCloneSerializer,
     CourseRunCreateSerializer,
     CourseRunImageSerializer,
     CourseRunRerunSerializer,
@@ -93,3 +94,10 @@ def rerun(self, request, *args, **kwargs):  # lint-amnesty, pylint: disable=miss
         new_course_run = serializer.save()
         serializer = self.get_serializer(new_course_run)
         return Response(serializer.data, status=status.HTTP_201_CREATED)
+
+    @action(detail=False, methods=['post'])
+    def clone(self, request, *args, **kwargs):  # lint-amnesty, pylint: disable=missing-function-docstring, unused-argument
+        serializer = CourseCloneSerializer(data=request.data, context=self.get_serializer_context())
+        serializer.is_valid(raise_exception=True)
+        serializer.save()
+        return Response({"message": "Course cloned successfully."}, status=status.HTTP_201_CREATED)

cms/djangoapps/contentstore/config/waffle.py

@@ -53,3 +53,13 @@
 # .. toggle_warning: Flag course_experience.relative_dates should also be active for relative dates functionalities to work.
 # .. toggle_tickets: https://openedx.atlassian.net/browse/AA-844
 CUSTOM_RELATIVE_DATES = CourseWaffleFlag(f'{WAFFLE_NAMESPACE}.custom_relative_dates', __name__)
+
+# .. toggle_name: studio.prevent_staff_structure_deletion
+# .. toggle_implementation: WaffleFlag
+# .. toggle_default: False
+# .. toggle_description: Prevents staff from deleting course structures
+# .. toggle_use_cases: opt_in
+# .. toggle_creation_date: 2021-06-25
+PREVENT_STAFF_STRUCTURE_DELETION = WaffleFlag(
+    f'{WAFFLE_NAMESPACE}.prevent_staff_structure_deletion', __name__, LOG_PREFIX
+)

cms/djangoapps/contentstore/permissions.py

@@ -0,0 +1,10 @@
+"""
+Permission definitions for the contentstore djangoapp
+"""
+
+from bridgekeeper import perms
+
+from lms.djangoapps.courseware.rules import HasRolesRule
+
+DELETE_COURSE_CONTENT = 'contentstore.delete_course_content'
+perms[DELETE_COURSE_CONTENT] = HasRolesRule('instructor')

cms/djangoapps/contentstore/rest_api/v0/tests/test_details_settings.py

@@ -0,0 +1,74 @@
+"""
+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)

cms/djangoapps/contentstore/rest_api/v0/urls.py

@@ -3,7 +3,13 @@
 from django.urls import re_path
 
 from openedx.core.constants import COURSE_ID_PATTERN
-from .views import AdvancedCourseSettingsView, CourseTabSettingsView, CourseTabListView, CourseTabReorderView
+from .views import (
+    AdvancedCourseSettingsView,
+    CourseDetailsSettingsView,
+    CourseTabSettingsView,
+    CourseTabListView,
+    CourseTabReorderView
+)
 
 app_name = "v0"
 
@@ -28,4 +34,9 @@
         CourseTabReorderView.as_view(),
         name="course_tab_reorder",
     ),
+    re_path(
+        fr"^details_settings/{COURSE_ID_PATTERN}$",
+        CourseDetailsSettingsView.as_view(),
+        name="course_details_settings",
+    ),
 ]

cms/djangoapps/contentstore/rest_api/v0/views/__init__.py

@@ -2,4 +2,5 @@
 Views for v0 contentstore API.
 """
 from .advanced_settings import AdvancedCourseSettingsView
+from .details_settings import CourseDetailsSettingsView
 from .tabs import CourseTabSettingsView, CourseTabListView, CourseTabReorderView

cms/djangoapps/contentstore/rest_api/v0/views/details_settings.py

@@ -0,0 +1,69 @@
+""" 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)

cms/djangoapps/contentstore/tests/test_libraries.py

@@ -16,7 +16,7 @@
 
 from cms.djangoapps.contentstore.tests.utils import AjaxEnabledTestClient, parse_json
 from cms.djangoapps.contentstore.utils import reverse_library_url, reverse_url, reverse_usage_url
-from cms.djangoapps.contentstore.views.block import _duplicate_block
+from cms.djangoapps.contentstore.views.block import duplicate_block
 from cms.djangoapps.contentstore.views.preview import _load_preview_block
 from cms.djangoapps.contentstore.views.tests.test_library import LIBRARY_REST_URL
 from cms.djangoapps.course_creators.views import add_user_with_status_granted
@@ -947,7 +947,7 @@ def test_persistent_overrides(self, duplicate):
         if duplicate:
             # Check that this also works when the RCB is duplicated.
             self.lc_block = modulestore().get_item(
-                _duplicate_block(self.course.location, self.lc_block.location, self.user)
+                duplicate_block(self.course.location, self.lc_block.location, self.user)
             )
             self.problem_in_course = modulestore().get_item(self.lc_block.children[0])
         else:
@@ -1006,7 +1006,7 @@ def test_duplicated_version(self):
 
         # Duplicate self.lc_block:
         duplicate = store.get_item(
-            _duplicate_block(self.course.location, self.lc_block.location, self.user)
+            duplicate_block(self.course.location, self.lc_block.location, self.user)
         )
         # The duplicate should have identical children to the original:
         self.assertEqual(len(duplicate.children), 1)