diff --git a/cms/__init__.py b/cms/__init__.py index f9ed0bb3cea1..d1bf27534315 100644 --- a/cms/__init__.py +++ b/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 diff --git a/cms/djangoapps/api/v1/serializers/course_runs.py b/cms/djangoapps/api/v1/serializers/course_runs.py index cbd4d09e2181..6bbbce96dd42 100644 --- a/cms/djangoapps/api/v1/serializers/course_runs.py +++ b/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 diff --git a/cms/djangoapps/api/v1/tests/test_views/test_course_runs.py b/cms/djangoapps/api/v1/tests/test_views/test_course_runs.py index 49589a473878..8366ef72941e 100644 --- a/cms/djangoapps/api/v1/tests/test_views/test_course_runs.py +++ b/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.' diff --git a/cms/djangoapps/api/v1/views/course_runs.py b/cms/djangoapps/api/v1/views/course_runs.py index a0415d4e06dc..fb1671ebef04 100644 --- a/cms/djangoapps/api/v1/views/course_runs.py +++ b/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) diff --git a/cms/djangoapps/contentstore/config/waffle.py b/cms/djangoapps/contentstore/config/waffle.py index 8f431fa49327..16719e61bf29 100644 --- a/cms/djangoapps/contentstore/config/waffle.py +++ b/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 +) diff --git a/cms/djangoapps/contentstore/permissions.py b/cms/djangoapps/contentstore/permissions.py new file mode 100644 index 000000000000..14fe40c09ca7 --- /dev/null +++ b/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') 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 new file mode 100644 index 000000000000..a6e2d6b19aea --- /dev/null +++ b/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) diff --git a/cms/djangoapps/contentstore/rest_api/v0/urls.py b/cms/djangoapps/contentstore/rest_api/v0/urls.py index eb435ef338d8..d2db1471868b 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/urls.py +++ b/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", + ), ] diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/__init__.py b/cms/djangoapps/contentstore/rest_api/v0/views/__init__.py index 91f4388cd8db..1cc7a9ca249f 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/__init__.py +++ b/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 diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/details_settings.py b/cms/djangoapps/contentstore/rest_api/v0/views/details_settings.py new file mode 100644 index 000000000000..3d2392c9d132 --- /dev/null +++ b/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) diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py index 3e850e4794d7..13da5cee2b54 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/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) diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index c65aaace82d8..7305bc9adc89 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -32,7 +32,8 @@ from xblock.core import XBlock from xblock.fields import Scope -from cms.djangoapps.contentstore.config.waffle import SHOW_REVIEW_RULES_FLAG +from cms.djangoapps.contentstore.config.waffle import PREVENT_STAFF_STRUCTURE_DELETION, SHOW_REVIEW_RULES_FLAG +from cms.djangoapps.contentstore.permissions import DELETE_COURSE_CONTENT from cms.djangoapps.models.settings.course_grading import CourseGradingModel from cms.lib.xblock.authoring_mixin import VISIBILITY_VIEW from common.djangoapps.edxmako.services import MakoService @@ -241,11 +242,11 @@ def xblock_handler(request, usage_key_string=None): status=400 ) - dest_usage_key = _duplicate_block( + dest_usage_key = duplicate_block( parent_usage_key, duplicate_source_usage_key, request.user, - request.json.get('display_name'), + display_name=request.json.get('display_name'), ) return JsonResponse({ 'locator': str(dest_usage_key), @@ -878,47 +879,88 @@ def _move_item(source_usage_key, target_parent_usage_key, user, target_index=Non return JsonResponse(context) -def _duplicate_block(parent_usage_key, duplicate_source_usage_key, user, display_name=None, is_child=False): +def gather_block_attributes(source_item, display_name=None, is_child=False): + """ + Gather all the attributes of the source block that need to be copied over to a new or updated block. + """ + # Update the display name to indicate this is a duplicate (unless display name provided). + # Can't use own_metadata(), b/c it converts data for JSON serialization - + # not suitable for setting metadata of the new block + duplicate_metadata = {} + for field in source_item.fields.values(): + if field.scope == Scope.settings and field.is_set_on(source_item): + duplicate_metadata[field.name] = field.read_from(source_item) + + if is_child: + display_name = display_name or source_item.display_name or source_item.category + + if display_name is not None: + duplicate_metadata['display_name'] = display_name + else: + if source_item.display_name is None: + duplicate_metadata['display_name'] = _("Duplicate of {0}").format(source_item.category) + else: + duplicate_metadata['display_name'] = _("Duplicate of '{0}'").format(source_item.display_name) + + asides_to_create = [] + for aside in source_item.runtime.get_asides(source_item): + for field in aside.fields.values(): + if field.scope in (Scope.settings, Scope.content,) and field.is_set_on(aside): + asides_to_create.append(aside) + break + + for aside in asides_to_create: + for field in aside.fields.values(): + if field.scope not in (Scope.settings, Scope.content,): + field.delete_from(aside) + return duplicate_metadata, asides_to_create + + +def update_from_source(*, source_block, destination_block, user_id): + """ + Update a block to have all the settings and attributes of another source. + + Copies over all attributes and settings of a source block to a destination + block. Blocks must be the same type. This function does not modify or duplicate + children. + """ + duplicate_metadata, asides = gather_block_attributes(source_block, display_name=source_block.display_name) + for key, value in duplicate_metadata.items(): + setattr(destination_block, key, value) + for key, value in source_block.get_explicitly_set_fields_by_scope(Scope.content).items(): + setattr(destination_block, key, value) + modulestore().update_item( + destination_block, + user_id, + metadata=duplicate_metadata, + asides=asides, + ) + + +def duplicate_block( + parent_usage_key, + duplicate_source_usage_key, + user, + dest_usage_key=None, + display_name=None, + shallow=False, + is_child=False +): """ Duplicate an existing xblock as a child of the supplied parent_usage_key. """ store = modulestore() with store.bulk_operations(duplicate_source_usage_key.course_key): source_item = store.get_item(duplicate_source_usage_key) - # Change the blockID to be unique. - dest_usage_key = source_item.location.replace(name=uuid4().hex) - category = dest_usage_key.block_type + if not dest_usage_key: + # Change the blockID to be unique. + dest_usage_key = source_item.location.replace(name=uuid4().hex) - # Update the display name to indicate this is a duplicate (unless display name provided). - # Can't use own_metadata(), b/c it converts data for JSON serialization - - # not suitable for setting metadata of the new block - duplicate_metadata = {} - for field in source_item.fields.values(): - if field.scope == Scope.settings and field.is_set_on(source_item): - duplicate_metadata[field.name] = field.read_from(source_item) - - if is_child: - display_name = display_name or source_item.display_name or source_item.category - - if display_name is not None: - duplicate_metadata['display_name'] = display_name - else: - if source_item.display_name is None: - duplicate_metadata['display_name'] = _("Duplicate of {0}").format(source_item.category) - else: - duplicate_metadata['display_name'] = _("Duplicate of '{0}'").format(source_item.display_name) - - asides_to_create = [] - for aside in source_item.runtime.get_asides(source_item): - for field in aside.fields.values(): - if field.scope in (Scope.settings, Scope.content,) and field.is_set_on(aside): - asides_to_create.append(aside) - break + category = dest_usage_key.block_type - for aside in asides_to_create: - for field in aside.fields.values(): - if field.scope not in (Scope.settings, Scope.content,): - field.delete_from(aside) + duplicate_metadata, asides_to_create = gather_block_attributes( + source_item, display_name=display_name, is_child=is_child, + ) dest_block = store.create_item( user.id, @@ -942,10 +984,10 @@ def _duplicate_block(parent_usage_key, duplicate_source_usage_key, user, display # Children are not automatically copied over (and not all xblocks have a 'children' attribute). # Because DAGs are not fully supported, we need to actually duplicate each child as well. - if source_item.has_children and not children_handled: + if source_item.has_children and not shallow and not children_handled: dest_block.children = dest_block.children or [] for child in source_item.children: - dupe = _duplicate_block(dest_block.location, child, user=user, is_child=True) + dupe = duplicate_block(dest_block.location, child, user=user, is_child=True) if dupe not in dest_block.children: # _duplicate_block may add the child for us. dest_block.children.append(dupe) store.update_item(dest_block, user.id) @@ -1372,6 +1414,12 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F else: xblock_info['staff_only_message'] = False + xblock_info['show_delete_button'] = True + if PREVENT_STAFF_STRUCTURE_DELETION.is_enabled(): + xblock_info['show_delete_button'] = ( + user.has_perm(DELETE_COURSE_CONTENT, xblock) if user is not None else False + ) + xblock_info['has_partition_group_components'] = has_children_visible_to_specific_partition_groups( xblock ) diff --git a/cms/djangoapps/contentstore/views/certificates.py b/cms/djangoapps/contentstore/views/certificates.py index 6535e0a85718..d27a648c91e9 100644 --- a/cms/djangoapps/contentstore/views/certificates.py +++ b/cms/djangoapps/contentstore/views/certificates.py @@ -231,6 +231,8 @@ def serialize_certificate(certificate): # Some keys are not required, such as the title override... if certificate_data.get('course_title'): certificate_response["course_title"] = certificate_data['course_title'] + if certificate_data.get('course_description'): + certificate_response['course_description'] = certificate_data['course_description'] return certificate_response diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 023cb07199fe..17ce3eea18b1 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -74,6 +74,7 @@ from openedx.core.djangoapps.credit.tasks import update_credit_course_requirements from openedx.core.djangoapps.models.course_details import CourseDetails from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers +from openedx.core.djangoapps.user_api.models import UserPreference from openedx.core.djangolib.js_utils import dump_js_escaped_json from openedx.core.lib.course_tabs import CourseTabPluginManager from openedx.core.lib.courses import course_image_url @@ -1191,6 +1192,13 @@ def settings_handler(request, course_key_string): # lint-amnesty, pylint: disab verified_mode = CourseMode.verified_mode_for_course(course_key, include_expired=True) upgrade_deadline = (verified_mode and verified_mode.expiration_datetime and verified_mode.expiration_datetime.isoformat()) + + date_placeholder_format = configuration_helpers.get_value_for_org( + course_block.location.org, + 'SCHEDULE_DETAIL_FORMAT', + settings.SCHEDULE_DETAIL_FORMAT + ).upper() + settings_context = { 'context_course': course_block, 'course_locator': course_key, @@ -1215,6 +1223,7 @@ def settings_handler(request, course_key_string): # lint-amnesty, pylint: disab 'enable_extended_course_details': enable_extended_course_details, 'upgrade_deadline': upgrade_deadline, 'mfe_proctored_exam_settings_url': get_proctored_exam_settings_url(course_block.id), + 'date_placeholder_format': date_placeholder_format, } if is_prerequisite_courses_enabled(): courses, in_process_course_actions = get_courses_accessible_to_user(request) @@ -1249,6 +1258,12 @@ def settings_handler(request, course_key_string): # lint-amnesty, pylint: disab elif 'application/json' in request.META.get('HTTP_ACCEPT', ''): # pylint: disable=too-many-nested-blocks if request.method == 'GET': course_details = CourseDetails.fetch(course_key) + + # Fetch the prefered timezone setup by the user + # and pass it as part of Json response + user_timezone = UserPreference.get_value(request.user, 'time_zone') + course_details.user_timezone = user_timezone + return JsonResponse( course_details, # encoder serializes dates, old locations, and instances @@ -1256,63 +1271,70 @@ def settings_handler(request, course_key_string): # lint-amnesty, pylint: disab ) # For every other possible method type submitted by the caller... else: - # 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 + 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 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 @@ -1343,6 +1365,7 @@ def grading_handler(request, course_key_string, grader_index=None): '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 }) elif 'application/json' in request.META.get('HTTP_ACCEPT', ''): if request.method == 'GET': diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index 7f5cbfe2be85..716b0d9a70b5 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -16,6 +16,7 @@ from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED from openedx_events.tests.utils import OpenEdxEventsTestMixin from edx_proctoring.exceptions import ProctoredExamNotFoundException +from edx_toggles.toggles.testutils import override_waffle_flag from opaque_keys import InvalidKeyError from opaque_keys.edx.asides import AsideUsageKeyV2 from opaque_keys.edx.keys import CourseKey, UsageKey @@ -49,6 +50,8 @@ from cms.djangoapps.contentstore.tests.utils import CourseTestCase from cms.djangoapps.contentstore.utils import reverse_course_url, reverse_usage_url from cms.djangoapps.contentstore.views import block as item_module +from cms.djangoapps.contentstore.config.waffle import PREVENT_STAFF_STRUCTURE_DELETION +from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, CourseCreatorRole from common.djangoapps.student.tests.factories import StaffFactory, UserFactory from common.djangoapps.xblock_django.models import ( XBlockConfiguration, @@ -67,7 +70,7 @@ _get_source_index, _xblock_type_and_display_name, add_container_page_publishing_info, - create_xblock_info, + create_xblock_info, duplicate_block, update_from_source, ) @@ -786,6 +789,29 @@ def verify_name(source_usage_key, parent_usage_key, expected_name, display_name= # Now send a custom display name for the duplicate. verify_name(self.seq_usage_key, self.chapter_usage_key, "customized name", display_name="customized name") + def test_shallow_duplicate(self): + """ + Test that shallow_duplicate creates a new block. + """ + source_course = CourseFactory() + user = UserFactory.create() + source_chapter = BlockFactory(parent=source_course, category='chapter', display_name='Source Chapter') + BlockFactory(parent=source_chapter, category='html', display_name='Child') + # Refresh. + source_chapter = self.store.get_item(source_chapter.location) + self.assertEqual(len(source_chapter.get_children()), 1) + destination_course = CourseFactory() + destination_location = duplicate_block( + parent_usage_key=destination_course.location, duplicate_source_usage_key=source_chapter.location, + user=user, + display_name=source_chapter.display_name, + shallow=True, + ) + # Refresh here, too, just to be sure. + destination_chapter = self.store.get_item(destination_location) + self.assertEqual(len(destination_chapter.get_children()), 0) + self.assertEqual(destination_chapter.display_name, 'Source Chapter') + @ddt.ddt class TestMoveItem(ItemTest): @@ -3473,3 +3499,255 @@ def test_self_paced_item_visibility_state(self, store_type): # Check that in self paced course content has live state now xblock_info = self._get_xblock_info(chapter.location) self._verify_visibility_state(xblock_info, VisibilityState.live) + + def test_staff_show_delete_button(self): + """ + Test delete button is *not visible* to user with CourseStaffRole + """ + # Add user as course staff + CourseStaffRole(self.course_key).add_users(self.user) + + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=self.user + ) + self.assertTrue(xblock_info['show_delete_button']) + + def test_staff_show_delete_button_with_waffle(self): + """ + Test delete button is *not visible* to user with CourseStaffRole and + PREVENT_STAFF_STRUCTURE_DELETION waffle set + """ + # Add user as course staff + CourseStaffRole(self.course_key).add_users(self.user) + + with override_waffle_flag(PREVENT_STAFF_STRUCTURE_DELETION, active=True): + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=self.user + ) + + self.assertFalse(xblock_info['show_delete_button']) + + def test_no_user_show_delete_button(self): + """ + Test delete button is *visible* when user attribute is not set on + xblock. This happens with ajax requests. + """ + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=None + ) + self.assertTrue(xblock_info['show_delete_button']) + + def test_no_user_show_delete_button_with_waffle(self): + """ + Test delete button is *visible* when user attribute is not set on + xblock (this happens with ajax requests) and PREVENT_STAFF_STRUCTURE_DELETION waffle set. + """ + + with override_waffle_flag(PREVENT_STAFF_STRUCTURE_DELETION, active=True): + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=None + ) + + self.assertFalse(xblock_info['show_delete_button']) + + def test_instructor_show_delete_button(self): + """ + Test delete button is *visible* to user with CourseInstructorRole only + """ + # Add user as course instructor + CourseInstructorRole(self.course_key).add_users(self.user) + + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=self.user + ) + self.assertTrue(xblock_info['show_delete_button']) + + def test_instructor_show_delete_button_with_waffle(self): + """ + Test delete button is *visible* to user with CourseInstructorRole only + and PREVENT_STAFF_STRUCTURE_DELETION waffle set + """ + # Add user as course instructor + CourseInstructorRole(self.course_key).add_users(self.user) + + with override_waffle_flag(PREVENT_STAFF_STRUCTURE_DELETION, active=True): + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=self.user + ) + + self.assertTrue(xblock_info['show_delete_button']) + + def test_creator_show_delete_button(self): + """ + Test delete button is *visible* to user with CourseInstructorRole only + """ + # Add user as course creator + CourseCreatorRole(self.course_key).add_users(self.user) + + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=self.user + ) + self.assertTrue(xblock_info['show_delete_button']) + + def test_creator_show_delete_button_with_waffle(self): + """ + Test delete button is *visible* to user with CourseInstructorRole only + and PREVENT_STAFF_STRUCTURE_DELETION waffle set + """ + # Add user as course creator + CourseCreatorRole(self.course_key).add_users(self.user) + + with override_waffle_flag(PREVENT_STAFF_STRUCTURE_DELETION, active=True): + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=self.user + ) + + self.assertFalse(xblock_info['show_delete_button']) + + +@patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + lambda self, block: ['test_aside']) +class TestUpdateFromSource(ModuleStoreTestCase): + """ + Test update_from_source. + """ + + def setUp(self): + """ + Set up the runtime for tests. + """ + super().setUp() + key_store = DictKeyValueStore() + field_data = KvsFieldData(key_store) + self.runtime = TestRuntime(services={'field-data': field_data}) + + def create_source_block(self, course): + """ + Create a chapter with all the fixings. + """ + source_block = BlockFactory( + parent=course, + category='course_info', + display_name='Source Block', + metadata={'due': datetime(2010, 11, 22, 4, 0, tzinfo=UTC)}, + ) + + def_id = self.runtime.id_generator.create_definition('html') + usage_id = self.runtime.id_generator.create_usage(def_id) + + aside = AsideTest(scope_ids=ScopeIds('user', 'html', def_id, usage_id), runtime=self.runtime) + aside.field11 = 'html_new_value1' + + # The data attribute is handled in a special manner and should be updated. + source_block.data = '
test
' + # This field is set on the content scope (definition_data), which should be updated. + source_block.items = ['test', 'beep'] + + self.store.update_item(source_block, self.user.id, asides=[aside]) + + # quick sanity checks + source_block = self.store.get_item(source_block.location) + self.assertEqual(source_block.due, datetime(2010, 11, 22, 4, 0, tzinfo=UTC)) + self.assertEqual(source_block.display_name, 'Source Block') + self.assertEqual(source_block.runtime.get_asides(source_block)[0].field11, 'html_new_value1') + self.assertEqual(source_block.data, '
test
') + self.assertEqual(source_block.items, ['test', 'beep']) + + return source_block + + def check_updated(self, source_block, destination_key): + """ + Check that the destination block has been updated to match our source block. + """ + revised = self.store.get_item(destination_key) + self.assertEqual(source_block.display_name, revised.display_name) + self.assertEqual(source_block.due, revised.due) + self.assertEqual(revised.data, source_block.data) + self.assertEqual(revised.items, source_block.items) + + self.assertEqual( + revised.runtime.get_asides(revised)[0].field11, + source_block.runtime.get_asides(source_block)[0].field11, + ) + + @XBlockAside.register_temp_plugin(AsideTest, 'test_aside') + def test_update_from_source(self): + """ + Test that update_from_source updates the destination block. + """ + course = CourseFactory() + user = UserFactory.create() + + source_block = self.create_source_block(course) + + destination_block = BlockFactory(parent=course, category='course_info', display_name='Destination Problem') + update_from_source(source_block=source_block, destination_block=destination_block, user_id=user.id) + self.check_updated(source_block, destination_block.location) + + @XBlockAside.register_temp_plugin(AsideTest, 'test_aside') + def test_update_clobbers(self): + """ + Verify that our update clobbers everything. + """ + course = CourseFactory() + user = UserFactory.create() + + source_block = self.create_source_block(course) + + destination_block = BlockFactory( + parent=course, + category='course_info', + display_name='Destination Chapter', + metadata={'due': datetime(2025, 10, 21, 6, 5, tzinfo=UTC)}, + ) + + def_id = self.runtime.id_generator.create_definition('html') + usage_id = self.runtime.id_generator.create_usage(def_id) + aside = AsideTest(scope_ids=ScopeIds('user', 'html', def_id, usage_id), runtime=self.runtime) + aside.field11 = 'Other stuff' + destination_block.data = '
other stuff
' + destination_block.items = ['other stuff', 'boop'] + self.store.update_item(destination_block, user.id, asides=[aside]) + + update_from_source(source_block=source_block, destination_block=destination_block, user_id=user.id) + self.check_updated(source_block, destination_block.location) diff --git a/cms/envs/common.py b/cms/envs/common.py index 70b4f0781a0a..8bce01ff1c52 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -182,6 +182,11 @@ # templates. STUDIO_NAME = _("Your Platform Studio") STUDIO_SHORT_NAME = _("Studio") + +# .. setting_name: SCHEDULE_DETAIL_FORMAT +# .. setting_default: 'MM/DD/YYYY' +# .. setting_description: Settings to configure the date format in Schedule & Details page +SCHEDULE_DETAIL_FORMAT = 'MM/DD/YYYY' FEATURES = { 'GITHUB_PUSH': False, @@ -342,6 +347,9 @@ # Allow public account creation 'ALLOW_PUBLIC_ACCOUNT_CREATION': True, + # Allow showing the registration links + 'SHOW_REGISTRATION_LINKS': True, + # Whether or not the dynamic EnrollmentTrackUserPartition should be registered. 'ENABLE_ENROLLMENT_TRACK_USER_PARTITION': True, @@ -526,6 +534,17 @@ # .. toggle_creation_date: 2023-03-31 # .. toggle_tickets: https://github.com/openedx/edx-platform/pull/32015 'DISABLE_ADVANCED_SETTINGS': False, + + # .. toggle_name: FEATURES['ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID'] + # .. toggle_implementation: DjangoSetting + # .. toggle_default: False + # .. toggle_description: Whether to enable the legacy MD5 hashing algorithm to generate anonymous user id + # instead of the newer SHAKE128 hashing algorithm + # .. toggle_use_cases: open_edx + # .. toggle_creation_date: 2022-08-08 + # .. toggle_target_removal_date: None + # .. toggle_tickets: 'https://github.com/openedx/edx-platform/pull/30832' + 'ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID': False, } # .. toggle_name: ENABLE_COPPA_COMPLIANCE @@ -2249,6 +2268,20 @@ # Maximum of 6 retries before giving up. SOFTWARE_SECURE_RETRY_MAX_ATTEMPTS = 6 + +# .. toggle_name: TPA_AUTOMATIC_LOGOUT_ENABLED +# .. toggle_implementation: DjangoSetting +# .. toggle_default: False +# .. toggle_description: Redirect the user to the TPA logout URL if this flag is enabled, the +# TPA logout URL is configured, and the user logs in through TPA +# .. toggle_use_cases: open_edx +# .. toggle_warning: Enabling this toggle skips rendering logout.html, which is used to log the user out +# from the different IDAs. To ensure the user is logged out of all the IDAs be sure to redirect +# back to /logout after logging out of the TPA. +# .. toggle_creation_date: 2023-05-07 +# .. toggle_tickets: https://github.com/openedx/edx-platform/pull/32193 +TPA_AUTOMATIC_LOGOUT_ENABLED = False + ############## DJANGO-USER-TASKS ############## # How long until database records about the outcome of a task and its artifacts get deleted? @@ -2305,6 +2338,14 @@ # Rate limit for regrading tasks that a grading policy change can kick off POLICY_CHANGE_TASK_RATE_LIMIT = '900/h' +# .. setting_name: DEFAULT_GRADE_DESIGNATIONS +# .. setting_default: ['A', 'B', 'C', 'D'] +# .. setting_description: The default 'pass' grade cutoff designations to be used. The failure grade +# is always 'F' and should not be included in this list. +# .. setting_warning: The DEFAULT_GRADE_DESIGNATIONS list must have more than one designation, +# or else ['A', 'B', 'C', 'D'] will be used as the default grade designations. +DEFAULT_GRADE_DESIGNATIONS = ['A', 'B', 'C', 'D'] + ############## Settings for CourseGraph ############################ # .. setting_name: COURSEGRAPH_JOB_QUEUE diff --git a/cms/static/cms/js/spec/main.js b/cms/static/cms/js/spec/main.js index f5aa089b0438..384eb7b83350 100644 --- a/cms/static/cms/js/spec/main.js +++ b/cms/static/cms/js/spec/main.js @@ -47,6 +47,7 @@ 'jquery.simulate': 'xmodule_js/common_static/js/vendor/jquery.simulate', 'datepair': 'xmodule_js/common_static/js/vendor/timepicker/datepair', 'date': 'xmodule_js/common_static/js/vendor/date', + 'moment-timezone': 'common/js/vendor/moment-timezone-with-data', moment: 'common/js/vendor/moment-with-locales', 'text': 'xmodule_js/common_static/js/vendor/requirejs/text', 'underscore': 'common/js/vendor/underscore', diff --git a/cms/static/js/certificates/models/certificate.js b/cms/static/js/certificates/models/certificate.js index 97a2735d7159..2b2fbf6eaec0 100644 --- a/cms/static/js/certificates/models/certificate.js +++ b/cms/static/js/certificates/models/certificate.js @@ -18,6 +18,7 @@ function(_, Backbone, BackboneRelational, BackboneAssociations, gettext, CoffeeS defaults: { // Metadata fields currently displayed in web forms course_title: '', + course_description: '', // Metadata fields not currently displayed in web forms name: 'Name of the certificate', diff --git a/cms/static/js/certificates/views/certificate_editor.js b/cms/static/js/certificates/views/certificate_editor.js index 6b0365ffbf65..afe0e0c473d0 100644 --- a/cms/static/js/certificates/views/certificate_editor.js +++ b/cms/static/js/certificates/views/certificate_editor.js @@ -24,6 +24,7 @@ function($, _, Backbone, gettext, 'change .collection-name-input': 'setName', 'change .certificate-description-input': 'setDescription', 'change .certificate-course-title-input': 'setCourseTitle', + 'change .certificate-course-description-input': 'setCourseDescription', 'focus .input-text': 'onFocus', 'blur .input-text': 'onBlur', submit: 'setAndClose', @@ -103,6 +104,7 @@ function($, _, Backbone, gettext, name: this.model.get('name'), description: this.model.get('description'), course_title: this.model.get('course_title'), + course_description: this.model.get('course_description'), org_logo_path: this.model.get('org_logo_path'), is_active: this.model.get('is_active'), isNew: this.model.isNew() @@ -143,11 +145,22 @@ function($, _, Backbone, gettext, ); }, + setCourseDescription: function(event) { + // Updates the indicated model field (still requires persistence on server) + if (event && event.preventDefault) { event.preventDefault(); } + this.model.set( + 'course_description', + this.$('.certificate-course-description-input').val(), + {silent: true} + ); + }, + setValues: function() { // Update the specified values in the local model instance this.setName(); this.setDescription(); this.setCourseTitle(); + this.setCourseDescription(); return this; } }); diff --git a/cms/static/js/factories/settings_graders.js b/cms/static/js/factories/settings_graders.js index 2be17111e0e2..b59961acbd42 100644 --- a/cms/static/js/factories/settings_graders.js +++ b/cms/static/js/factories/settings_graders.js @@ -2,7 +2,7 @@ define([ 'jquery', 'js/views/settings/grading', 'js/models/settings/course_grading_policy' ], function($, GradingView, CourseGradingPolicyModel) { 'use strict'; - return function(courseDetails, gradingUrl) { + return function(courseDetails, gradingUrl, gradeDesignations) { var model, editor; $('form :input') @@ -17,7 +17,8 @@ define([ model.urlRoot = gradingUrl; editor = new GradingView({ el: $('.settings-grading'), - model: model + model: model, + gradeDesignations: gradeDesignations }); editor.render(); }; diff --git a/cms/static/js/spec/views/pages/course_outline_spec.js b/cms/static/js/spec/views/pages/course_outline_spec.js index 7b0abb53ee25..074a77c47324 100644 --- a/cms/static/js/spec/views/pages/course_outline_spec.js +++ b/cms/static/js/spec/views/pages/course_outline_spec.js @@ -41,7 +41,8 @@ describe('CourseOutlinePage', function() { user_partitions: [], user_partition_info: {}, highlights_enabled: true, - highlights_enabled_for_messaging: false + highlights_enabled_for_messaging: false, + show_delete_button: true }, options, {child_info: {children: children}}); }; @@ -68,7 +69,8 @@ describe('CourseOutlinePage', function() { show_review_rules: true, user_partition_info: {}, highlights_enabled: true, - highlights_enabled_for_messaging: false + highlights_enabled_for_messaging: false, + show_delete_button: true }, options, {child_info: {children: children}}); }; @@ -93,7 +95,8 @@ describe('CourseOutlinePage', function() { group_access: {}, user_partition_info: {}, highlights: [], - highlights_enabled: true + highlights_enabled: true, + show_delete_button: true }, options, {child_info: {children: children}}); }; @@ -123,7 +126,8 @@ describe('CourseOutlinePage', function() { }, user_partitions: [], group_access: {}, - user_partition_info: {} + user_partition_info: {}, + show_delete_button: true }, options, {child_info: {children: children}}); }; @@ -141,7 +145,8 @@ describe('CourseOutlinePage', function() { edited_by: 'MockUser', user_partitions: [], group_access: {}, - user_partition_info: {} + user_partition_info: {}, + show_delete_button: true }, options); }; @@ -862,6 +867,13 @@ describe('CourseOutlinePage', function() { expect(outlinePage.$('[data-locator="mock-section-2"]')).toExist(); }); + it('remains un-visible if show_delete_button is false ', function() { + createCourseOutlinePage(this, createMockCourseJSON({show_delete_button: false}, [ + createMockSectionJSON({show_delete_button: false}) + ])); + expect(getItemHeaders('section').find('.delete-button').first()).not.toExist(); + }); + it('can be deleted if it is the only section', function() { var promptSpy = EditHelpers.createPromptSpy(); createCourseOutlinePage(this, mockSingleSectionCourseJSON); @@ -2246,13 +2258,6 @@ describe('CourseOutlinePage', function() { it('shows validation error on relative date', function() { outlinePage.$('.outline-subsection .configure-button').click(); - // when due number of weeks goes over 18 - selectRelativeWeeksSubsection('19'); - expect($('#relative_weeks_due_warning_max').css('display')).not.toBe('none'); - expect($('#relative_weeks_due_warning_max')).toContainText('The maximum number of weeks this subsection can be due in is 18 weeks from the learner enrollment date.'); - expect($('.wrapper-modal-window .action-save').prop('disabled')).toBe(true); - expect($('.wrapper-modal-window .action-save').hasClass('is-disabled')).toBe(true); - // when due number of weeks is less than 1 selectRelativeWeeksSubsection('-1'); expect($('#relative_weeks_due_warning_min').css('display')).not.toBe('none'); @@ -2261,8 +2266,7 @@ describe('CourseOutlinePage', function() { expect($('.wrapper-modal-window .action-save').hasClass('is-disabled')).toBe(true); // when no validation error should show up - selectRelativeWeeksSubsection('10'); - expect($('#relative_weeks_due_warning_max').css('display')).toBe('none'); + selectRelativeWeeksSubsection('19'); expect($('#relative_weeks_due_warning_min').css('display')).toBe('none'); expect($('.wrapper-modal-window .action-save').prop('disabled')).toBe(false); expect($('.wrapper-modal-window .action-save').hasClass('is-disabled')).toBe(false); diff --git a/cms/static/js/utils/date_utils.js b/cms/static/js/utils/date_utils.js index a335adb01b62..f8e71dff018e 100644 --- a/cms/static/js/utils/date_utils.js +++ b/cms/static/js/utils/date_utils.js @@ -1,5 +1,5 @@ -define(['jquery', 'date', 'js/utils/change_on_enter', 'jquery.ui', 'jquery.timepicker'], - function($, date, TriggerChangeEventOnEnter) { +define(['jquery', 'date', 'js/utils/change_on_enter', 'moment-timezone', 'jquery.ui', 'jquery.timepicker'], + function($, date, TriggerChangeEventOnEnter, moment) { 'use strict'; function getDate(datepickerInput, timepickerInput) { @@ -67,15 +67,51 @@ define(['jquery', 'date', 'js/utils/change_on_enter', 'jquery.ui', 'jquery.timep return obj; } - function setupDatePicker(fieldName, view, index) { + /** + * Calculates the utc offset in miliseconds for given + * timezone and subtracts it from given localized time + * to get time in UTC + * + * @param {Date} localTime JS Date object in Local Time + * @param {string} timezone IANA timezone name ex. "Australia/Brisbane" + * @returns JS Date object in UTC + */ + function convertLocalizedDateToUTC(localTime, timezone) { + const localTimeMS = localTime.getTime(); + const utcOffset = moment.tz(localTime, timezone)._offset; + return new Date(localTimeMS - (utcOffset * 60 *1000)); + } + + /** + * Returns the timezone abbreviation for given + * timezone name + * + * @param {string} timezone IANA timezone name ex. "Australia/Brisbane" + * @returns Timezone abbreviation ex. "AEST" + */ + function getTZAbbreviation(timezone) { + return moment(new Date()).tz(timezone).format('z'); + } + + /** + * Converts the given datetime string from UTC to localized time + * + * @param {string} utcDateTime JS Date object with UTC datetime + * @param {string} timezone IANA timezone name ex. "Australia/Brisbane" + * @returns Formatted datetime string with localized timezone + */ + function getLocalizedCurrentDate(utcDateTime, timezone) { + const localDateTime = moment(utcDateTime).tz(timezone); + return localDateTime.format('YYYY-MM-DDTHH[:]mm[:]ss'); + }function setupDatePicker(fieldName, view, index) { var cacheModel; var div; var datefield; - var timefield; + var timefield;var tzfield; var cacheview; var setfield; var currentDate; - if (typeof index !== 'undefined' && view.hasOwnProperty('collection')) { + var timezone;if (typeof index !== 'undefined' && view.hasOwnProperty('collection')) { cacheModel = view.collection.models[index]; div = view.$el.find('#' + view.collectionSelector(cacheModel.cid)); } else { @@ -84,11 +120,16 @@ define(['jquery', 'date', 'js/utils/change_on_enter', 'jquery.ui', 'jquery.timep } datefield = $(div).find('input.date'); timefield = $(div).find('input.time'); - cacheview = view; + tzfield = $(div).find('span.timezone'); + cacheview = view; + + timezone = cacheModel.get('user_timezone'); setfield = function(event) { var newVal = getDate(datefield, timefield); - // Setting to null clears the time as well, as date and time are linked. + if (timezone) { + newVal = convertLocalizedDateToUTC(newVal, timezone); + }// Setting to null clears the time as well, as date and time are linked. // Note also that the validation logic prevents us from clearing the start date // (start date is required by the back end). cacheview.clearValidationErrors(); @@ -97,7 +138,12 @@ define(['jquery', 'date', 'js/utils/change_on_enter', 'jquery.ui', 'jquery.timep // instrument as date and time pickers timefield.timepicker({timeFormat: 'H:i'}); - datefield.datepicker(); + var placeholder = datefield.attr('placeholder'); + if (placeholder == 'DD/MM/YYYY') { + datefield.datepicker({dateFormat: 'dd/mm/yy'}); + } else { + datefield.datepicker(); + } // Using the change event causes setfield to be triggered twice, but it is necessary // to pick up when the date is typed directly in the field. @@ -109,8 +155,13 @@ define(['jquery', 'date', 'js/utils/change_on_enter', 'jquery.ui', 'jquery.timep if (cacheModel) { currentDate = cacheModel.get(fieldName); } - // timepicker doesn't let us set null, so check that we have a time - if (currentDate) { + if (timezone) { + const tz = getTZAbbreviation(timezone); + $(tzfield).text("("+tz+")"); + } // timepicker doesn't let us set null, so check that we have a time + if (currentDate) {if (timezone) { + currentDate = getLocalizedCurrentDate(currentDate, timezone); + } setDate(datefield, timefield, currentDate); } else { // but reset fields either way diff --git a/cms/static/js/views/modals/course_outline_modals.js b/cms/static/js/views/modals/course_outline_modals.js index 855af813a256..a10a4f23749d 100644 --- a/cms/static/js/views/modals/course_outline_modals.js +++ b/cms/static/js/views/modals/course_outline_modals.js @@ -420,16 +420,11 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', validateDueIn: function() { this.$('#relative_weeks_due_projected').hide(); - if (this.getValue() > 18){ - this.$('#relative_weeks_due_warning_max').show(); - BaseModal.prototype.disableActionButton.call(this.parent, 'save'); - } - else if (this.getValue() < 1){ - this.$('#relative_weeks_due_warning_min').show() + if (this.getValue() < 1){ + this.$('#relative_weeks_due_warning_min').show(); BaseModal.prototype.disableActionButton.call(this.parent, 'save'); } else { - this.$('#relative_weeks_due_warning_max').hide(); this.$('#relative_weeks_due_warning_min').hide(); this.showProjectedDate(); BaseModal.prototype.enableActionButton.call(this.parent, 'save'); @@ -455,7 +450,7 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', // Grab all the subsections, map them to their block_ids, then return as an Array var subsectionIds = $('.outline-subsection').map(function(){return this.id;}).get() var relative_weeks_due = null; - if (this.getValue() < 19 && this.getValue() > 0 && $('#grading_type').val() !== 'notgraded') { + if (this.getValue() > 0 && $('#grading_type').val() !== 'notgraded') { relative_weeks_due = this.getValue() } window.analytics.track('edx.bi.studio.relative_date.saved', { diff --git a/cms/static/js/views/settings/grading.js b/cms/static/js/views/settings/grading.js index 4320204035ac..684545cc730e 100644 --- a/cms/static/js/views/settings/grading.js +++ b/cms/static/js/views/settings/grading.js @@ -24,7 +24,7 @@ function(ValidatingView, _, $, ui, GraderView, StringUtils, HtmlUtils) { 'focus :input': 'inputFocus', 'blur :input': 'inputUnfocus' }, - initialize: function() { + initialize: function(options) { // load template for grading view var self = this; this.template = HtmlUtils.template( @@ -34,6 +34,7 @@ function(ValidatingView, _, $, ui, GraderView, StringUtils, HtmlUtils) { $('#course_grade_cutoff-tpl').text() ); this.setupCutoffs(); + this.setupGradeDesignations(options.gradeDesignations); this.listenTo(this.model, 'invalid', this.handleValidationError); this.listenTo(this.model, 'change', this.showNotificationBar); @@ -297,7 +298,7 @@ function(ValidatingView, _, $, ui, GraderView, StringUtils, HtmlUtils) { addNewGrade: function(e) { e.preventDefault(); var gradeLength = this.descendingCutoffs.length; // cutoffs doesn't include fail/f so this is only the passing grades - if (gradeLength > 3) { + if (gradeLength > this.GRADES.length - 1) { // TODO shouldn't we disable the button return; } @@ -377,7 +378,9 @@ function(ValidatingView, _, $, ui, GraderView, StringUtils, HtmlUtils) { this.descendingCutoffs = _.sortBy(this.descendingCutoffs, function(gradeEle) { return -gradeEle.cutoff; }); }, - revertView: function() { + setupGradeDesignations: function(gradeDesignations) { + if (Array.isArray(gradeDesignations) && gradeDesignations.length > 1) { this.GRADES = gradeDesignations; } + },revertView: function() { var self = this; this.model.fetch({ success: function() { diff --git a/cms/static/js/views/xblock_outline.js b/cms/static/js/views/xblock_outline.js index 1f1538e502fe..37d7881045f1 100644 --- a/cms/static/js/views/xblock_outline.js +++ b/cms/static/js/views/xblock_outline.js @@ -109,8 +109,8 @@ function($, _, gettext, BaseView, ViewUtils, XBlockViewUtils, XBlockStringFieldE includesChildren: this.shouldRenderChildren(), hasExplicitStaffLock: this.model.get('has_explicit_staff_lock'), staffOnlyMessage: this.model.get('staff_only_message'), - course: course - }; + course: course, + showDeleteButton: this.model.get('show_delete_button')}; }, renderChildren: function() { diff --git a/cms/static/sass/views/_settings.scss b/cms/static/sass/views/_settings.scss index 10d2ccc3272a..a786f6f2082a 100644 --- a/cms/static/sass/views/_settings.scss +++ b/cms/static/sass/views/_settings.scss @@ -777,23 +777,23 @@ height: 17px; } - &:nth-child(1) { + &:nth-child(5n+1) { background: #4fe696; } - &:nth-child(2) { + &:nth-child(5n+2) { background: #ffdf7e; } - &:nth-child(3) { + &:nth-child(5n+3) { background: #ffb657; } - &:nth-child(4) { + &:nth-child(5n+4) { background: #ef54a1; } - &:nth-child(5), + &:nth-child(5n+5), &.bar-fail { background: #fb336c; } diff --git a/cms/templates/howitworks.html b/cms/templates/howitworks.html index b04a207eba6f..d21dff2d66a5 100644 --- a/cms/templates/howitworks.html +++ b/cms/templates/howitworks.html @@ -154,7 +154,7 @@

${_("Work in Teams")}

-% if static.get_value('ALLOW_PUBLIC_ACCOUNT_CREATION', settings.FEATURES.get('ALLOW_PUBLIC_ACCOUNT_CREATION')): +% if static.get_value('ALLOW_PUBLIC_ACCOUNT_CREATION', settings.FEATURES.get('ALLOW_PUBLIC_ACCOUNT_CREATION')) and settings.FEATURES.get('SHOW_REGISTRATION_LINKS', True):
diff --git a/cms/templates/js/certificate-details.underscore b/cms/templates/js/certificate-details.underscore index a09a3baf897c..3401fd175a39 100644 --- a/cms/templates/js/certificate-details.underscore +++ b/cms/templates/js/certificate-details.underscore @@ -29,6 +29,12 @@ <%- course_title %>

<% } %> + <% if (course_description) { %> +

+ <%- gettext('Course Description') %>: + <%- course_description %> +

+ <% } %>
diff --git a/cms/templates/js/certificate-editor.underscore b/cms/templates/js/certificate-editor.underscore index 513113b80500..3b1d90969b5a 100644 --- a/cms/templates/js/certificate-editor.underscore +++ b/cms/templates/js/certificate-editor.underscore @@ -31,6 +31,11 @@ " value="<%- course_title %>" aria-describedby="certificate-course-title-<%-uniqueId %>-tip" /> <%- gettext("Specify an alternative to the official course title to display on certificates. Leave blank to use the official course title.") %>
+
+ + " value="<%- course_description %>" aria-describedby="certificate-course-description-<%-uniqueId %>-tip" /> + <%- gettext("Specify an alternative to the official course description to display on certificates. Leave blank to use default text.") %> +

<%- gettext("Certificate Signatories") %>

diff --git a/cms/templates/js/course-outline.underscore b/cms/templates/js/course-outline.underscore index 4550d102a6ad..7f82410196bb 100644 --- a/cms/templates/js/course-outline.underscore +++ b/cms/templates/js/course-outline.underscore @@ -179,7 +179,7 @@ if (is_proctored_exam) { <% } %> - <% if (xblockInfo.isDeletable()) { %> + <% if (xblockInfo.isDeletable() && showDeleteButton) { %>
  • diff --git a/cms/templates/js/self-paced-due-date-editor.underscore b/cms/templates/js/self-paced-due-date-editor.underscore index ceaf5e6873ce..086471aa4ba6 100644 --- a/cms/templates/js/self-paced-due-date-editor.underscore +++ b/cms/templates/js/self-paced-due-date-editor.underscore @@ -4,7 +4,7 @@ + placeholder="" autocomplete="off" min="1" style="width:20%"/> <%- gettext('weeks from learner enrollment date')%>
  • @@ -19,10 +19,6 @@ %> -
    - <%- gettext('The maximum number of weeks this subsection can be due in is 18 weeks from the learner enrollment date.') %> -
    -
    <%- gettext('The minimum number of weeks this subsection can be due in is 1 week from the learner enrollment date.') %>
    diff --git a/cms/templates/settings.html b/cms/templates/settings.html index b7ef368a0911..2f99cbfa0c2b 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -224,7 +224,7 @@

    ${_('Course Schedule')}

  • - + ${_("First day the course begins")}
    @@ -239,7 +239,7 @@

    ${_('Course Schedule')}

  • - + ${_("Last day your course is active")}
    @@ -305,8 +305,9 @@

    ${_('Course Schedule')}

    % endif - + + ${_("By default, 48 hours after course end date")}
  • @@ -316,7 +317,7 @@

    ${_('Course Schedule')}

  • - + ${_("First day students can enroll")}
    @@ -334,7 +335,7 @@

    ${_('Course Schedule')}

  • - + ${_("Last day students can enroll.")} @@ -357,7 +358,7 @@

    ${_('Course Schedule')}

  • - + ${_("Last day students can upgrade to a verified enrollment.")} ${_("Contact your {platform_name} partner manager to update these settings.").format(platform_name=settings.PLATFORM_NAME)} diff --git a/cms/templates/settings_graders.html b/cms/templates/settings_graders.html index ac7119177978..d8b7d3bc0ebd 100644 --- a/cms/templates/settings_graders.html +++ b/cms/templates/settings_graders.html @@ -34,7 +34,8 @@ SettingsGradersFactory( _.extend(${dump_js_escaped_json(course_details, cls=CourseSettingsEncoder) | n, decode.utf8}, {is_credit_course: ${is_credit_course | n, dump_js_escaped_json}}), - "${grading_url | n, js_escaped_string}" + "${grading_url | n, js_escaped_string}", + ${default_grade_designations | n, dump_js_escaped_json}, ); }); diff --git a/cms/templates/widgets/header.html b/cms/templates/widgets/header.html index 48b818da95a5..6f110caf2e88 100644 --- a/cms/templates/widgets/header.html +++ b/cms/templates/widgets/header.html @@ -268,7 +268,7 @@

    ${_("Account Navigation")}

  • - % if static.get_value('ALLOW_PUBLIC_ACCOUNT_CREATION', settings.FEATURES.get('ALLOW_PUBLIC_ACCOUNT_CREATION')): + % if static.get_value('ALLOW_PUBLIC_ACCOUNT_CREATION', settings.FEATURES.get('ALLOW_PUBLIC_ACCOUNT_CREATION')) and settings.FEATURES.get('SHOW_REGISTRATION_LINKS', True): diff --git a/common/djangoapps/student/models/user.py b/common/djangoapps/student/models/user.py index f29480e0ce02..f9f60c16ddd9 100644 --- a/common/djangoapps/student/models/user.py +++ b/common/djangoapps/student/models/user.py @@ -150,12 +150,21 @@ def anonymous_id_for_user(user, course_id): # function: Rotate at will, since the hashes are stored and # will not change. # include the secret key as a salt, and to make the ids unique across different LMS installs. - hasher = hashlib.shake_128() + legacy_hash_enabled = settings.FEATURES.get('ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID', False) + if legacy_hash_enabled: + # Use legacy MD5 algorithm if flag enabled + hasher = hashlib.md5() + else: + hasher = hashlib.shake_128() hasher.update(settings.SECRET_KEY.encode('utf8')) hasher.update(str(user.id).encode('utf8')) if course_id: hasher.update(str(course_id).encode('utf-8')) - anonymous_user_id = hasher.hexdigest(16) + + if legacy_hash_enabled: + anonymous_user_id = hasher.hexdigest() + else: + anonymous_user_id = hasher.hexdigest(16) # pylint: disable=too-many-function-args try: AnonymousUserId.objects.create( diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 1fcd9887ea07..3749bc05035a 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -120,7 +120,7 @@ class GlobalStaff(AccessRole): The global staff role """ def has_user(self, user): - return bool(user and user.is_staff) + return bool(user and (user.is_superuser or user.is_staff)) def add_users(self, *users): for user in users: diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index b41ad2f856d6..8f952831f12b 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -15,6 +15,7 @@ from django.test import TestCase, override_settings from django.test.client import Client from django.urls import reverse +from edx_toggles.toggles.testutils import override_waffle_switch from markupsafe import escape from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locations import CourseLocator @@ -33,6 +34,7 @@ user_by_anonymous_id ) from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory +from common.djangoapps.student.toggles import REDIRECT_TO_COURSEWARE_AFTER_ENROLLMENT from common.djangoapps.student.views import complete_course_mode_info from common.djangoapps.util.model_utils import USER_SETTINGS_CHANGED_EVENT_NAME from common.djangoapps.util.testing import EventTestMixin @@ -893,6 +895,7 @@ def test_change_enrollment_modes(self): @skip_unless_lms +@ddt.ddt class ChangeEnrollmentViewTest(ModuleStoreTestCase): """Tests the student.views.change_enrollment view""" @@ -913,6 +916,17 @@ def _enroll_through_view(self, course): ) return response + @ddt.data( + (True, 'courseware'), + (False, None), + ) + @ddt.unpack + def test_enrollment_url(self, waffle_flag_enabled, returned_view): + with override_waffle_switch(REDIRECT_TO_COURSEWARE_AFTER_ENROLLMENT, waffle_flag_enabled): + response = self._enroll_through_view(self.course) + data = reverse(returned_view, args=[str(self.course.id)]) if returned_view else '' + assert response.content.decode('utf8') == data + def test_enroll_as_default(self): """Tests that a student can successfully enroll through this view""" response = self._enroll_through_view(self.course) @@ -1050,6 +1064,17 @@ def test_anonymous_id_secret_key_changes_result_in_diff_values_for_same_new_user assert anonymous_id != new_anonymous_id assert self.user == user_by_anonymous_id(new_anonymous_id) + def test_enable_legacy_hash_flag(self): + """Test that different anonymous id returned if ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID enabled.""" + CourseEnrollment.enroll(self.user, self.course.id) + anonymous_id = anonymous_id_for_user(self.user, self.course.id) + with patch.dict(settings.FEATURES, ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID=True): + # Recreate user object to clear cached anonymous id. + self.user = User.objects.get(pk=self.user.id) + AnonymousUserId.objects.filter(user=self.user).filter(course_id=self.course.id).delete() + new_anonymous_id = anonymous_id_for_user(self.user, self.course.id) + assert anonymous_id != new_anonymous_id + @skip_unless_lms @patch('openedx.core.djangoapps.programs.utils.get_programs') diff --git a/common/djangoapps/student/toggles.py b/common/djangoapps/student/toggles.py index 46e9b0408fbf..4b773449e5c9 100644 --- a/common/djangoapps/student/toggles.py +++ b/common/djangoapps/student/toggles.py @@ -2,6 +2,7 @@ Toggles for Dashboard page. """ from edx_toggles.toggles import WaffleFlag +from edx_toggles.toggles import WaffleSwitch # Namespace for student waffle flags. WAFFLE_FLAG_NAMESPACE = 'student' @@ -75,3 +76,20 @@ def should_show_2u_recommendations(): def should_send_enrollment_email(): return ENROLLMENT_CONFIRMATION_EMAIL.is_enabled() + +# Waffle flag to enable control redirecting after enrolment. +# .. toggle_name: student.redirect_to_courseware_after_enrollment +# .. toggle_implementation: WaffleSwitch +# .. toggle_default: False +# .. toggle_description: Redirect to courseware after enrollment instead of dashboard. +# .. toggle_use_cases: open_edx +# .. toggle_creation_date: 2023-02-06 +# .. toggle_target_removal_date: None +# .. toggle_warning: None +REDIRECT_TO_COURSEWARE_AFTER_ENROLLMENT = WaffleSwitch( + f'{WAFFLE_FLAG_NAMESPACE}.redirect_to_courseware_after_enrollment', __name__ +) + + +def should_redirect_to_courseware_after_enrollment(): + return REDIRECT_TO_COURSEWARE_AFTER_ENROLLMENT.is_enabled() diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index d131575193b4..cb3df1627fd9 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -38,6 +38,7 @@ from rest_framework.decorators import api_view, authentication_classes, permission_classes from rest_framework.permissions import IsAuthenticated +from common.djangoapps.student.toggles import should_redirect_to_courseware_after_enrollment from common.djangoapps.track import views as track_views from lms.djangoapps.bulk_email.models import Optout from common.djangoapps.course_modes.models import CourseMode @@ -400,8 +401,10 @@ def change_enrollment(request, check_access=True): reverse("course_modes_choose", kwargs={'course_id': str(course_id)}) ) - # Otherwise, there is only one mode available (the default) - return HttpResponse() + if should_redirect_to_courseware_after_enrollment(): + return HttpResponse(reverse('courseware', args=[str(course_id)])) + else: + return HttpResponse() elif action == "unenroll": if configuration_helpers.get_value( "DISABLE_UNENROLLMENT", diff --git a/common/djangoapps/util/json_request.py b/common/djangoapps/util/json_request.py index 3c9646b130e5..d71620020771 100644 --- a/common/djangoapps/util/json_request.py +++ b/common/djangoapps/util/json_request.py @@ -8,6 +8,8 @@ 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): @@ -40,7 +42,6 @@ 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 @@ -54,7 +55,14 @@ def parse_json_into_request(request, *args, **kwargs): return view_function(request, *args, **kwargs) - return parse_json_into_request + 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 class JsonResponse(HttpResponse): diff --git a/lms/__init__.py b/lms/__init__.py index 008640ac7147..05a30f4ffad4 100644 --- a/lms/__init__.py +++ b/lms/__init__.py @@ -18,3 +18,30 @@ # that shared_task will use this app, and also ensures that the celery # singleton is always configured for the LMS. from .celery import APP as CELERY_APP # lint-amnesty, pylint: disable=wrong-import-position + +# 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 + + +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 diff --git a/lms/djangoapps/certificates/tests/test_webview_views.py b/lms/djangoapps/certificates/tests/test_webview_views.py index 96832e3cb694..a61fcd6dd7c4 100644 --- a/lms/djangoapps/certificates/tests/test_webview_views.py +++ b/lms/djangoapps/certificates/tests/test_webview_views.py @@ -140,6 +140,7 @@ def _add_course_certificates(self, count=1, signatory_count=0, is_active=True): 'name': 'Name ' + str(i), 'description': 'Description ' + str(i), 'course_title': 'course_title_' + str(i), + 'course_description': 'course_description_' + str(i), 'org_logo_path': f'/t4x/orgX/testX/asset/org-logo-{i}.png', 'signatories': signatories, 'version': 1, @@ -460,11 +461,6 @@ def test_rendering_course_organization_data(self): uuid=self.cert.verify_uuid ) response = self.client.get(test_url) - self.assertContains( - response, - 'a course of study offered by test_organization, an online learning initiative of test organization', - ) - self.assertNotContains(response, 'a course of study offered by testorg') self.assertContains(response, f'test_organization {self.course.number} Certificate |') self.assertContains(response, 'logo_test1.png') @@ -549,21 +545,13 @@ def test_rendering_maximum_data(self): self.assertContains(response, '<a class="logo" href="http://test_site.localhost">') # Test an item from course info self.assertContains(response, 'course_title_0') + # Test an item from course description + self.assertContains(response, 'course_description_0') # Test an item from user info self.assertContains(response, f"{self.user.profile.name}, you earned a certificate!") # Test an item from social info self.assertContains(response, "Post on Facebook") self.assertContains(response, "Share on Twitter") - # Test an item from certificate/org info - self.assertContains( - response, - "a course of study offered by {partner_short_name}, " - "an online learning initiative of " - "{partner_long_name}.".format( - partner_short_name=short_org_name, - partner_long_name=long_org_name, - ), - ) # Test item from badge info self.assertContains(response, "Add to Mozilla Backpack") # Test item from site configuration diff --git a/lms/djangoapps/certificates/views/webview.py b/lms/djangoapps/certificates/views/webview.py index b859cfe92ca2..e9f9afd34470 100644 --- a/lms/djangoapps/certificates/views/webview.py +++ b/lms/djangoapps/certificates/views/webview.py @@ -254,7 +254,10 @@ def _update_course_context(request, context, course, platform_name): course_number = course.display_coursenumber if course.display_coursenumber else course.number context['course_number'] = course_number context['idv_enabled_for_certificates'] = settings.FEATURES.get('ENABLE_CERTIFICATES_IDV_REQUIREMENT') - if context['organization_long_name']: + course_description_override = context['certificate_data'].get('course_description', '') + if course_description_override: + context['accomplishment_copy_course_description'] = course_description_override + elif context['organization_long_name']: # Translators: This text represents the description of course context['accomplishment_copy_course_description'] = _('a course of study offered by {partner_short_name}, ' 'an online learning initiative of ' diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 54adcb3e3638..bb6b69eb9f03 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -590,6 +590,7 @@ def test_instructor_level(self): @patch.dict(settings.FEATURES, {'ALLOW_AUTOMATED_SIGNUPS': True}) +@ddt.ddt class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCase, LoginEnrollmentTestCase): """ Test Bulk account creation and enrollment from csv file @@ -641,32 +642,15 @@ def setUp(self): ) @patch('lms.djangoapps.instructor.views.api.log.info') - def test_account_creation_and_enrollment_with_csv(self, info_log): - """ - Happy path test to create a single new user - """ - csv_content = b"test_student@example.com,test_student_1,tester1,USA" - uploaded_file = SimpleUploadedFile("temp.csv", csv_content) - response = self.client.post(self.url, {'students_list': uploaded_file, 'email-students': True}) - assert response.status_code == 200 - data = json.loads(response.content.decode('utf-8')) - assert len(data['row_errors']) == 0 - assert len(data['warnings']) == 0 - assert len(data['general_errors']) == 0 - - manual_enrollments = ManualEnrollmentAudit.objects.all() - assert manual_enrollments.count() == 1 - assert manual_enrollments[0].state_transition == UNENROLLED_TO_ENROLLED - - # test the log for email that's send to new created user. - info_log.assert_called_with('email sent to new created user at %s', 'test_student@example.com') - - @patch('lms.djangoapps.instructor.views.api.log.info') - def test_account_creation_and_enrollment_with_csv_with_blank_lines(self, info_log): + @ddt.data( + b"test_student@example.com,test_student_1,tester1,USA", # Typical use case. + b"\ntest_student@example.com,test_student_1,tester1,USA\n\n", # Blank lines. + b"\xef\xbb\xbftest_student@example.com,test_student_1,tester1,USA", # Unicode signature (BOM). + ) + def test_account_creation_and_enrollment_with_csv(self, csv_content, info_log): """ Happy path test to create a single new user """ - csv_content = b"\ntest_student@example.com,test_student_1,tester1,USA\n\n" uploaded_file = SimpleUploadedFile("temp.csv", csv_content) response = self.client.post(self.url, {'students_list': uploaded_file, 'email-students': True}) assert response.status_code == 200 @@ -4385,7 +4369,7 @@ def call_add_users_to_cohorts(self, csv_data, suffix='.csv'): """ # this temporary file will be removed in `self.tearDown()` __, file_name = tempfile.mkstemp(suffix=suffix, dir=self.tempdir) - with open(file_name, 'w') as file_pointer: + with open(file_name, 'w', encoding='utf-8-sig') as file_pointer: file_pointer.write(csv_data) with open(file_name) as file_pointer: url = reverse('add_users_to_cohorts', kwargs={'course_id': str(self.course.id)}) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index eb0d6aa8a5a7..c83af8e1a430 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -339,7 +339,7 @@ def register_and_enroll_students(request, course_id): # pylint: disable=too-man try: upload_file = request.FILES.get('students_list') if upload_file.name.endswith('.csv'): - students = list(csv.reader(upload_file.read().decode('utf-8').splitlines())) + students = list(csv.reader(upload_file.read().decode('utf-8-sig').splitlines())) course = get_course_by_id(course_id) else: general_errors.append({ @@ -1519,7 +1519,7 @@ def _cohorts_csv_validator(file_storage, file_to_validate): Verifies that the expected columns are present in the CSV used to add users to cohorts. """ with file_storage.open(file_to_validate) as f: - reader = csv.reader(f.read().decode('utf-8').splitlines()) + reader = csv.reader(f.read().decode('utf-8-sig').splitlines()) try: fieldnames = next(reader) @@ -3332,7 +3332,7 @@ def build_row_errors(key, _user, row_count): try: upload_file = request.FILES.get('students_list') if upload_file.name.endswith('.csv'): - students = list(csv.reader(upload_file.read().decode('utf-8').splitlines())) + students = list(csv.reader(upload_file.read().decode('utf-8-sig').splitlines())) else: general_errors.append(_('Make sure that the file you upload is in CSV format with no ' 'extraneous characters or rows.')) diff --git a/lms/envs/common.py b/lms/envs/common.py index c0182d47355a..23c335cee154 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -777,6 +777,15 @@ # .. toggle_tickets: https://openedx.atlassian.net/browse/YONK-513 'ALLOW_PUBLIC_ACCOUNT_CREATION': True, + # .. toggle_name: FEATURES['SHOW_REGISTRATION_LINKS'] + # .. toggle_implementation: DjangoSetting + # .. toggle_default: True + # .. toggle_description: Allow registration links. If this is disabled, users will no longer see buttons to the + # the signup page. + # .. toggle_use_cases: open_edx + # .. toggle_creation_date: 2023-03-27 + 'SHOW_REGISTRATION_LINKS': True, + # .. toggle_name: FEATURES['ENABLE_COOKIE_CONSENT'] # .. toggle_implementation: DjangoSetting # .. toggle_default: False @@ -1028,6 +1037,17 @@ # .. toggle_creation_date: 2022-06-06 # .. toggle_tickets: 'https://github.com/edx/edx-platform/pull/29538' 'DISABLE_ALLOWED_ENROLLMENT_IF_ENROLLMENT_CLOSED': False, + + # .. toggle_name: FEATURES['ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID'] + # .. toggle_implementation: DjangoSetting + # .. toggle_default: False + # .. toggle_description: Whether to enable the legacy MD5 hashing algorithm to generate anonymous user id + # instead of the newer SHAKE128 hashing algorithm + # .. toggle_use_cases: open_edx + # .. toggle_creation_date: 2022-08-08 + # .. toggle_target_removal_date: None + # .. toggle_tickets: 'https://github.com/openedx/edx-platform/pull/30832' + 'ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID': False, } # Specifies extra XBlock fields that should available when requested via the Course Blocks API @@ -1215,6 +1235,19 @@ TPA_PROVIDER_BURST_THROTTLE = '10/min' TPA_PROVIDER_SUSTAINED_THROTTLE = '50/hr' +# .. toggle_name: TPA_AUTOMATIC_LOGOUT_ENABLED +# .. toggle_implementation: DjangoSetting +# .. toggle_default: False +# .. toggle_description: Redirect the user to the TPA logout URL if this flag is enabled, the +# TPA logout URL is configured, and the user logs in through TPA. +# .. toggle_use_cases: open_edx +# .. toggle_warning: Enabling this toggle skips rendering logout.html, which is used to log the user out +# from the different IDAs. To ensure the user is logged out of all the IDAs be sure to redirect +# back to <LMS>/logout after logging out of the TPA. +# .. toggle_creation_date: 2023-05-07 +# .. toggle_tickets: https://github.com/openedx/edx-platform/pull/32193 +TPA_AUTOMATIC_LOGOUT_ENABLED = False + ################################## TEMPLATE CONFIGURATION ##################################### # Mako templating import tempfile # pylint: disable=wrong-import-position,wrong-import-order diff --git a/lms/static/js/student_account/views/AccessView.js b/lms/static/js/student_account/views/AccessView.js index 7a2e1a858d36..6c5b3d2be696 100644 --- a/lms/static/js/student_account/views/AccessView.js +++ b/lms/static/js/student_account/views/AccessView.js @@ -75,7 +75,8 @@ this.platformName = options.platform_name; this.supportURL = options.support_link; this.passwordResetSupportUrl = options.password_reset_support_link; - this.createAccountOption = options.account_creation_allowed; + this.createAccountOption = options.account_creation_allowed && options.register_links_allowed; + this.showRegisterLinks = options.register_links_allowed this.hideAuthWarnings = options.hide_auth_warnings || false; this.pipelineUserDetails = options.third_party_auth.pipeline_user_details; this.enterpriseName = options.enterprise_name || ''; @@ -161,7 +162,7 @@ supportURL: this.supportURL, passwordResetSupportUrl: this.passwordResetSupportUrl, createAccountOption: this.createAccountOption, - hideAuthWarnings: this.hideAuthWarnings, + showRegisterLinks: this.showRegisterLinks,hideAuthWarnings: this.hideAuthWarnings, pipelineUserDetails: this.pipelineUserDetails, enterpriseName: this.enterpriseName, enterpriseSlugLoginURL: this.enterpriseSlugLoginURL, @@ -186,8 +187,9 @@ this.subview.passwordHelp = new PasswordResetView({ fields: data.fields, - model: this.resetModel - }); + model: this.resetModel, + showRegisterLinks: this.showRegisterLinks + }); // Listen for 'password-email-sent' event to toggle sub-views this.listenTo(this.subview.passwordHelp, 'password-email-sent', this.passwordEmailSent); @@ -211,7 +213,8 @@ hideAuthWarnings: this.hideAuthWarnings, is_require_third_party_auth_enabled: this.is_require_third_party_auth_enabled, enableCoppaCompliance: this.enable_coppa_compliance, - }); + showRegisterLinks: this.showRegisterLinks + }); // Listen for 'auth-complete' event so we can enroll/redirect the user appropriately. this.listenTo(this.subview.register, 'auth-complete', this.authComplete); diff --git a/lms/static/js/student_account/views/FormView.js b/lms/static/js/student_account/views/FormView.js index d5cbb75868bd..5a9a269612af 100644 --- a/lms/static/js/student_account/views/FormView.js +++ b/lms/static/js/student_account/views/FormView.js @@ -35,6 +35,7 @@ initialize: function(data) { this.model = data.model; + this.showRegisterLinks = data.showRegisterLinks; this.preRender(data); this.tpl = $(this.tpl).html(); @@ -97,6 +98,7 @@ supplementalText: data[i].supplementalText || '', supplementalLink: data[i].supplementalLink || '', loginIssueSupportLink: data[i].loginIssueSupportLink || '', + showRegisterLinks: this.showRegisterLinks, isEnterpriseEnable: this.isEnterpriseEnable }))); } diff --git a/lms/static/js/student_account/views/LoginView.js b/lms/static/js/student_account/views/LoginView.js index 10c6bc309cdd..3fe530db907a 100644 --- a/lms/static/js/student_account/views/LoginView.js +++ b/lms/static/js/student_account/views/LoginView.js @@ -52,6 +52,7 @@ this.supportURL = data.supportURL; this.passwordResetSupportUrl = data.passwordResetSupportUrl; this.createAccountOption = data.createAccountOption; + this.showRegisterLinks = data.showRegisterLinks; this.accountActivationMessages = data.accountActivationMessages; this.accountRecoveryMessages = data.accountRecoveryMessages; this.hideAuthWarnings = data.hideAuthWarnings; @@ -212,6 +213,7 @@ saveError: function(error) { var errorCode; var msg; + var redirectURL; if (error.status === 0) { msg = gettext('An error has occurred. Check your Internet connection and try again.'); } else if (error.status === 500) { @@ -241,6 +243,7 @@ } else if (error.responseJSON !== undefined) { msg = error.responseJSON.value; errorCode = error.responseJSON.error_code; + redirectURL = error.responseJSON.redirect_url; } else { msg = gettext('An unexpected error has occurred.'); } @@ -262,6 +265,7 @@ this.clearFormErrors(); this.renderThirdPartyAuthWarning(); } + window.location.href = redirectURL; } else { this.renderErrors(this.defaultFormErrorsTitle, this.errors); } diff --git a/lms/static/js/student_account/views/RegisterView.js b/lms/static/js/student_account/views/RegisterView.js index af6353ac407a..f463c6bfd2f6 100644 --- a/lms/static/js/student_account/views/RegisterView.js +++ b/lms/static/js/student_account/views/RegisterView.js @@ -88,8 +88,8 @@ requiredStr: this.requiredStr, optionalStr: fields[i].name === 'marketing_emails_opt_in' ? '' : this.optionalStr, supplementalText: fields[i].supplementalText || '', - supplementalLink: fields[i].supplementalLink || '' - }))); + supplementalLink: fields[i].supplementalLink || '', + showRegisterLinks: this.showRegisterLinks}))); } html.push('</div>'); return html; diff --git a/lms/templates/header/navbar-not-authenticated.html b/lms/templates/header/navbar-not-authenticated.html index 47aa9abd2ced..945e494bb0a7 100644 --- a/lms/templates/header/navbar-not-authenticated.html +++ b/lms/templates/header/navbar-not-authenticated.html @@ -19,7 +19,7 @@ courses_are_browsable = settings.FEATURES.get('COURSES_ARE_BROWSABLE') allows_login = not settings.FEATURES['DISABLE_LOGIN_BUTTON'] and not combined_login_and_register can_discover_courses = settings.FEATURES.get('ENABLE_COURSE_DISCOVERY') - allow_public_account_creation = static.get_value('ALLOW_PUBLIC_ACCOUNT_CREATION', settings.FEATURES.get('ALLOW_PUBLIC_ACCOUNT_CREATION')) + allow_public_account_creation = static.get_value('ALLOW_PUBLIC_ACCOUNT_CREATION', settings.FEATURES.get('ALLOW_PUBLIC_ACCOUNT_CREATION')) and settings.FEATURES.get('SHOW_REGISTRATION_LINKS', True) should_redirect_to_authn_mfe = should_redirect_to_authn_microfrontend() %> <nav class="nav-links" aria-label=${_("Supplemental Links")}> diff --git a/lms/templates/navigation/navbar-not-authenticated.html b/lms/templates/navigation/navbar-not-authenticated.html index a82a24e87834..418419550929 100644 --- a/lms/templates/navigation/navbar-not-authenticated.html +++ b/lms/templates/navigation/navbar-not-authenticated.html @@ -34,7 +34,7 @@ <a class="btn" href="/courses">${_("Explore Courses")}</a> </li> %endif - % if static.get_value('ALLOW_PUBLIC_ACCOUNT_CREATION', settings.FEATURES.get('ALLOW_PUBLIC_ACCOUNT_CREATION')): + % if static.get_value('ALLOW_PUBLIC_ACCOUNT_CREATION', settings.FEATURES.get('ALLOW_PUBLIC_ACCOUNT_CREATION')) and settings.FEATURES.get('SHOW_REGISTRATION_LINKS', True): <li class="item nav-global-04"> <a class="btn btn-neutral btn-register" href="/register${login_query()}">${_("Register")}</a> </li> diff --git a/lms/templates/student_account/form_field.underscore b/lms/templates/student_account/form_field.underscore index e64f9af4e4c4..bcd488ae24a2 100644 --- a/lms/templates/student_account/form_field.underscore +++ b/lms/templates/student_account/form_field.underscore @@ -147,10 +147,12 @@ <button type="button" class="enterprise-login field-link"><%- gettext("Sign in with your company or school") %></button> <% } %> <% } %> - <% if( form === 'password-reset' && name === 'email' ) { %> + <% if( (showRegisterLinks || loginIssueSupportLink) && form === 'password-reset' && name === 'email' ) { %> <button type="button" class="reset-help field-link" ><i class="fa fa-caret-right" /><%- gettext("Need other help signing in?") %></button> <div id="reset-help" style="display:none"> - <button type="button" class="field-link form-toggle" data-type="register"><%- gettext("Create an account") %></button> + <% if ( showRegisterLinks ) { %> + <button type="button" class="field-link form-toggle" data-type="register"><%- gettext("Create an account") %></button> + <% } %> <% if ( loginIssueSupportLink ) { %> <span><a class="field-link" href="<%- loginIssueSupportLink %>"><%- gettext("Other sign-in issues") %></a></span> <% } %> diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 79cb78a2727c..5c5fe63e3ef6 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -76,7 +76,7 @@ def _do_third_party_auth(request): try: return pipeline.get_authenticated_user(requested_provider, username, third_party_uid) - except USER_MODEL.DoesNotExist: + except USER_MODEL.DoesNotExist as err: AUDIT_LOG.info( "Login failed - user with username {username} has no social auth " "with backend_name {backend_name}".format( @@ -98,7 +98,13 @@ def _do_third_party_auth(request): ) ) - raise AuthFailedError(message, error_code='third-party-auth-with-no-linked-account') # lint-amnesty, pylint: disable=raise-missing-from + redirect_url = configuration_helpers.get_value('OC_REDIRECT_ON_TPA_UNLINKED_ACCOUNT', None) + + raise AuthFailedError( + message, + error_code='third-party-auth-with-no-linked-account', + redirect_url=redirect_url + ) from err def _get_user_by_email(email): diff --git a/openedx/core/djangoapps/user_authn/views/login_form.py b/openedx/core/djangoapps/user_authn/views/login_form.py index ee04123ddf9a..bb78a9df1a3c 100644 --- a/openedx/core/djangoapps/user_authn/views/login_form.py +++ b/openedx/core/djangoapps/user_authn/views/login_form.py @@ -258,6 +258,7 @@ def login_and_registration_form(request, initial_mode="login"): 'password_reset_form_desc': json.loads(form_descriptions['password_reset']), 'account_creation_allowed': configuration_helpers.get_value( 'ALLOW_PUBLIC_ACCOUNT_CREATION', settings.FEATURES.get('ALLOW_PUBLIC_ACCOUNT_CREATION', True)), + 'register_links_allowed': settings.FEATURES.get('SHOW_REGISTRATION_LINKS', True), 'is_account_recovery_feature_enabled': is_secondary_email_feature_enabled(), 'enterprise_slug_login_url': get_enterprise_slug_login_url(), 'is_enterprise_enable': enterprise_enabled(), diff --git a/openedx/core/djangoapps/user_authn/views/logout.py b/openedx/core/djangoapps/user_authn/views/logout.py index c00fa2a5dada..1fd9a3e47e8c 100644 --- a/openedx/core/djangoapps/user_authn/views/logout.py +++ b/openedx/core/djangoapps/user_authn/views/logout.py @@ -8,6 +8,7 @@ import bleach from django.conf import settings from django.contrib.auth import logout +from django.shortcuts import redirect from django.utils.http import urlencode from django.views.generic import TemplateView from oauth2_provider.models import Application @@ -83,6 +84,17 @@ def dispatch(self, request, *args, **kwargs): delete_logged_in_cookies(response) mark_user_change_as_expected(None) + + # Redirect to tpa_logout_url if TPA_AUTOMATIC_LOGOUT_ENABLED is set to True and if + # tpa_logout_url is configured. + # + # NOTE: This step skips rendering logout.html, which is used to log the user out from the + # different IDAs. To ensure the user is logged out of all the IDAs be sure to redirect + # back to <LMS>/logout after logging out of the TPA. + if settings.TPA_AUTOMATIC_LOGOUT_ENABLED: + if self.tpa_logout_url: + return redirect(self.tpa_logout_url) + return response def _build_logout_url(self, url): diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_logout.py b/openedx/core/djangoapps/user_authn/views/tests/test_logout.py index 139491d03d99..7d10fe1021ef 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_logout.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_logout.py @@ -196,6 +196,33 @@ def test_learner_portal_logout_having_idp_logout_url(self): } self.assertDictContainsSubset(expected, response.context_data) + @mock.patch('django.conf.settings.TPA_AUTOMATIC_LOGOUT_ENABLED', True) + def test_automatic_tpa_logout_url_redirect(self): + """ + Test user automatically redirected to tpa logout_url + when TPA_AUTOMATIC_LOGOUT is set to True. + """ + idp_logout_url = 'http://mock-idp.com/logout' + client = self._create_oauth_client() + + with mock.patch( + 'openedx.core.djangoapps.user_authn.views.logout.tpa_pipeline.get_idp_logout_url_from_running_pipeline' + ) as mock_idp_logout_url: + mock_idp_logout_url.return_value = idp_logout_url + self._authenticate_with_oauth(client) + response = self.client.get(reverse('logout')) + assert response.status_code == 302 + assert response.url == idp_logout_url + + @mock.patch('django.conf.settings.TPA_AUTOMATIC_LOGOUT_ENABLED', True) + def test_no_automatic_tpa_logout_without_logout_url(self): + """ + Test user is NOT automatically redirected when tpa logout_url is not set + even if TPA_AUTOMATIC_LOGOUT is set to True. + """ + client = self._create_oauth_client() + self._assert_session_logged_out(client) + @ddt.data( ('%22%3E%3Cscript%3Ealert(%27xss%27)%3C/script%3E', 'edx.org'), ) diff --git a/openedx/features/course_experience/__init__.py b/openedx/features/course_experience/__init__.py index 52b870033a26..df8179623593 100644 --- a/openedx/features/course_experience/__init__.py +++ b/openedx/features/course_experience/__init__.py @@ -50,6 +50,16 @@ # .. toggle_tickets: https://openedx.atlassian.net/browse/AA-27 RELATIVE_DATES_FLAG = CourseWaffleFlag(f'{WAFFLE_FLAG_NAMESPACE}.relative_dates', __name__) # lint-amnesty, pylint: disable=toggle-missing-annotation +# .. toggle_name: course_experience.relative_dates_disable_reset +# .. toggle_implementation: CourseWaffleFlag +# .. toggle_default: False +# .. toggle_description: Waffle flag to disable resetting deadlines by learners in self-paced courses. The 'Dates' tab +# will no longer show a banner about missed deadlines. The deadlines banner will also be hidden on unit pages. +# .. toggle_use_cases: open_edx +# .. toggle_creation_date: 2023-04-27 +# .. toggle_warning: For this toggle to have an effect, the RELATIVE_DATES_FLAG toggle must be enabled, too. +RELATIVE_DATES_DISABLE_RESET_FLAG = CourseWaffleFlag(f'{WAFFLE_FLAG_NAMESPACE}.relative_dates_disable_reset', __name__) + # .. toggle_name: course_experience.calendar_sync # .. toggle_implementation: CourseWaffleFlag # .. toggle_default: False diff --git a/openedx/features/course_experience/api/v1/tests/test_views.py b/openedx/features/course_experience/api/v1/tests/test_views.py index f67768c38f76..afc671dbf91f 100644 --- a/openedx/features/course_experience/api/v1/tests/test_views.py +++ b/openedx/features/course_experience/api/v1/tests/test_views.py @@ -2,17 +2,19 @@ Tests for reset deadlines endpoint. """ import datetime -import ddt +import ddt from django.urls import reverse from django.utils import timezone +from edx_toggles.toggles.testutils import override_waffle_flag from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.util.testing import EventTestMixin -from lms.djangoapps.courseware.tests.helpers import MasqueradeMixin from lms.djangoapps.course_home_api.tests.utils import BaseCourseHomeTests +from lms.djangoapps.courseware.tests.helpers import MasqueradeMixin from openedx.core.djangoapps.schedules.models import Schedule +from openedx.features.course_experience import RELATIVE_DATES_DISABLE_RESET_FLAG, RELATIVE_DATES_FLAG from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order @@ -25,17 +27,22 @@ def setUp(self): # pylint: disable=arguments-differ # Need to supply tracker name for the EventTestMixin. Also, EventTestMixin needs to come # first in class inheritance so the setUp call here appropriately works super().setUp('openedx.features.course_experience.api.v1.views.tracker') + self.course = CourseFactory.create(self_paced=True, start=timezone.now() - datetime.timedelta(days=1000)) def test_reset_deadlines(self): - CourseEnrollment.enroll(self.user, self.course.id, CourseMode.VERIFIED) + enrollment = CourseEnrollment.enroll(self.user, self.course.id, CourseMode.VERIFIED) + enrollment.schedule.start_date = timezone.now() - datetime.timedelta(days=100) + enrollment.schedule.save() # Test body with incorrect body param (course_key is required) response = self.client.post(reverse('course-experience-reset-course-deadlines'), {'course': self.course.id}) assert response.status_code == 400 + assert enrollment.schedule == Schedule.objects.get(id=enrollment.schedule.id) self.assert_no_events_were_emitted() # Test correct post body response = self.client.post(reverse('course-experience-reset-course-deadlines'), {'course_key': self.course.id}) assert response.status_code == 200 + assert enrollment.schedule.start_date < Schedule.objects.get(id=enrollment.schedule.id).start_date self.assert_event_emitted( 'edx.ui.lms.reset_deadlines.clicked', courserun_key=str(self.course.id), @@ -45,33 +52,44 @@ def test_reset_deadlines(self): user_id=self.user.id, ) + @override_waffle_flag(RELATIVE_DATES_FLAG, active=True) + @override_waffle_flag(RELATIVE_DATES_DISABLE_RESET_FLAG, active=True) + def test_reset_deadlines_disabled(self): + enrollment = CourseEnrollment.enroll(self.user, self.course.id, CourseMode.VERIFIED) + enrollment.schedule.start_date = timezone.now() - datetime.timedelta(days=100) + enrollment.schedule.save() + + response = self.client.post(reverse('course-experience-reset-course-deadlines'), {'course_key': self.course.id}) + assert response.status_code == 200 + assert enrollment.schedule == Schedule.objects.get(id=enrollment.schedule.id) + self.assert_no_events_were_emitted() + def test_reset_deadlines_with_masquerade(self): """ Staff users should be able to masquerade as a learner and reset the learner's schedule """ - course = CourseFactory.create(self_paced=True, start=timezone.now() - datetime.timedelta(days=1)) student_username = self.user.username student_user_id = self.user.id - student_enrollment = CourseEnrollment.enroll(self.user, course.id) + student_enrollment = CourseEnrollment.enroll(self.user, self.course.id) student_enrollment.schedule.start_date = timezone.now() - datetime.timedelta(days=100) student_enrollment.schedule.save() - staff_enrollment = CourseEnrollment.enroll(self.staff_user, course.id) + staff_enrollment = CourseEnrollment.enroll(self.staff_user, self.course.id) staff_enrollment.schedule.start_date = timezone.now() - datetime.timedelta(days=30) staff_enrollment.schedule.save() self.switch_to_staff() - self.update_masquerade(course=course, username=student_username) + self.update_masquerade(course=self.course, username=student_username) - self.client.post(reverse('course-experience-reset-course-deadlines'), {'course_key': course.id}) + self.client.post(reverse('course-experience-reset-course-deadlines'), {'course_key': self.course.id}) updated_schedule = Schedule.objects.get(id=student_enrollment.schedule.id) assert updated_schedule.start_date.date() == datetime.datetime.today().date() updated_staff_schedule = Schedule.objects.get(id=staff_enrollment.schedule.id) assert updated_staff_schedule.start_date == staff_enrollment.schedule.start_date self.assert_event_emitted( 'edx.ui.lms.reset_deadlines.clicked', - courserun_key=str(course.id), + courserun_key=str(self.course.id), is_masquerading=True, is_staff=False, - org_key=course.org, + org_key=self.course.org, user_id=student_user_id, ) diff --git a/openedx/features/course_experience/utils.py b/openedx/features/course_experience/utils.py index 451d769eeade..d58b54f6139f 100644 --- a/openedx/features/course_experience/utils.py +++ b/openedx/features/course_experience/utils.py @@ -9,7 +9,7 @@ from lms.djangoapps.course_blocks.api import get_course_blocks from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.lib.cache_utils import request_cached -from openedx.features.course_experience import RELATIVE_DATES_FLAG +from openedx.features.course_experience import RELATIVE_DATES_DISABLE_RESET_FLAG, RELATIVE_DATES_FLAG from common.djangoapps.student.models import CourseEnrollment from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order @@ -155,6 +155,14 @@ def dates_banner_should_display(course_key, user): if not RELATIVE_DATES_FLAG.is_enabled(course_key): return False, False + if RELATIVE_DATES_DISABLE_RESET_FLAG.is_enabled(course_key): + # The `missed_deadlines` value is ignored by `reset_course_deadlines` views. Instead, they check the value of + # `missed_gated_content` to determine if learners can reset the deadlines by themselves. + # We could have added this logic directly to `reset_self_paced_schedule`, but this function is used in other + # places (e.g., when an enrollment mode is changed). We want this flag to affect only the use case when + # learners try to reset their deadlines. + return False, True + course_overview = CourseOverview.objects.get(id=str(course_key)) # Only display the banner for self-paced courses diff --git a/setup.cfg b/setup.cfg index 7b1be0be92e8..fc5a2dee3e83 100644 --- a/setup.cfg +++ b/setup.cfg @@ -73,3 +73,4 @@ ignore_imports = cms.djangoapps.contentstore.views.preview -> lms.djangoapps.lms_xblock.field_data cms.envs.common -> lms.djangoapps.lms_xblock.mixin cms.envs.test -> lms.envs.test + cms.djangoapps.contentstore.permissions -> lms.djangoapps.courseware.rules diff --git a/xmodule/modulestore/split_mongo/split.py b/xmodule/modulestore/split_mongo/split.py index 8e7999ae75bc..7982ba46723c 100644 --- a/xmodule/modulestore/split_mongo/split.py +++ b/xmodule/modulestore/split_mongo/split.py @@ -57,7 +57,6 @@ import copy import datetime -import hashlib import logging from collections import defaultdict from importlib import import_module @@ -102,7 +101,7 @@ ) from xmodule.modulestore.split_mongo import BlockKey, CourseEnvelope from xmodule.modulestore.split_mongo.mongo_connection import DuplicateKeyError, DjangoFlexPersistenceBackend -from xmodule.modulestore.store_utilities import DETACHED_XBLOCK_TYPES +from xmodule.modulestore.store_utilities import DETACHED_XBLOCK_TYPES, derived_key from xmodule.partitions.partitions_service import PartitionService from xmodule.util.misc import get_library_or_course_attribute @@ -2438,7 +2437,6 @@ def _copy_from_template( for usage_key in source_keys: src_course_key = usage_key.course_key - hashable_source_id = src_course_key.for_version(None) block_key = BlockKey(usage_key.block_type, usage_key.block_id) source_structure = source_structures[src_course_key] @@ -2446,15 +2444,7 @@ def _copy_from_template( raise ItemNotFoundError(usage_key) source_block_info = source_structure['blocks'][block_key] - # Compute a new block ID. This new block ID must be consistent when this - # method is called with the same (source_key, dest_structure) pair - unique_data = "{}:{}:{}".format( - str(hashable_source_id).encode("utf-8"), - block_key.id, - new_parent_block_key.id, - ) - new_block_id = hashlib.sha1(unique_data.encode('utf-8')).hexdigest()[:20] - new_block_key = BlockKey(block_key.type, new_block_id) + new_block_key = derived_key(src_course_key, block_key, new_parent_block_key) # Now clone block_key to new_block_key: new_block_info = copy.deepcopy(source_block_info) diff --git a/xmodule/modulestore/store_utilities.py b/xmodule/modulestore/store_utilities.py index 45082ff43e75..964b4b459832 100644 --- a/xmodule/modulestore/store_utilities.py +++ b/xmodule/modulestore/store_utilities.py @@ -1,5 +1,5 @@ # lint-amnesty, pylint: disable=missing-module-docstring - +import hashlib import logging import re import uuid @@ -7,6 +7,8 @@ from xblock.core import XBlock +from xmodule.modulestore.split_mongo import BlockKey + DETACHED_XBLOCK_TYPES = {name for name, __ in XBlock.load_tagged_classes("detached")} @@ -104,3 +106,26 @@ def get_draft_subtree_roots(draft_nodes): for draft_node in draft_nodes: if draft_node.parent_url not in urls: yield draft_node + + +def derived_key(courselike_source_key, block_key, parent): + """ + Return a new reproducible block ID for a given root, source block, and destination parent. + + When recursively copying a block structure, we need to generate new block IDs for the + blocks. We don't want to use the exact same IDs as we might copy blocks multiple times. + However, we do want to be able to reproduce the same IDs when copying the same block + so that if we ever need to re-copy the block from its source (that is, to update it with + upstream changes) we don't affect any data tied to the ID, such as grades. + """ + hashable_source_id = courselike_source_key.for_version(None) + + # Compute a new block ID. This new block ID must be consistent when this + # method is called with the same (source_key, dest_structure) pair + unique_data = "{}:{}:{}".format( + str(hashable_source_id).encode("utf-8"), + block_key.id, + parent.id, + ) + new_block_id = hashlib.sha1(unique_data.encode('utf-8')).hexdigest()[:20] + return BlockKey(block_key.type, new_block_id) diff --git a/xmodule/modulestore/tests/test_store_utilities.py b/xmodule/modulestore/tests/test_store_utilities.py index 1c1c86f4fcd3..a0b5cfcbdbb6 100644 --- a/xmodule/modulestore/tests/test_store_utilities.py +++ b/xmodule/modulestore/tests/test_store_utilities.py @@ -4,11 +4,15 @@ import unittest +from unittest import TestCase from unittest.mock import Mock import ddt -from xmodule.modulestore.store_utilities import draft_node_constructor, get_draft_subtree_roots +from opaque_keys.edx.keys import CourseKey + +from xmodule.modulestore.split_mongo import BlockKey +from xmodule.modulestore.store_utilities import draft_node_constructor, get_draft_subtree_roots, derived_key @ddt.ddt @@ -82,3 +86,43 @@ def test_get_draft_subtree_roots(self, node_arguments_list, expected_roots_urls) subtree_roots_urls = [root.url for root in get_draft_subtree_roots(block_nodes)] # check that we return the expected urls assert set(subtree_roots_urls) == set(expected_roots_urls) + + +mock_block = Mock() +mock_block.id = CourseKey.from_string('course-v1:Beeper+B33P+BOOP') + + +derived_key_scenarios = [ + { + 'courselike_source_key': CourseKey.from_string('course-v1:edX+DemoX+Demo_Course'), + 'block_key': BlockKey('chapter', 'interactive_demonstrations'), + 'parent': mock_block, + 'expected': BlockKey( + 'chapter', '5793ec64e25ed870a7dd', + ), + }, + { + 'courselike_source_key': CourseKey.from_string('course-v1:edX+DemoX+Demo_Course'), + 'block_key': BlockKey('chapter', 'interactive_demonstrations'), + 'parent': BlockKey( + 'chapter', 'thingy', + ), + 'expected': BlockKey( + 'chapter', '599792a5622d85aa41e6', + ), + } +] + + +@ddt.ddt +class TestDerivedKey(TestCase): + """ + Test reproducible block ID generation. + """ + @ddt.data(*derived_key_scenarios) + @ddt.unpack + def test_derived_key(self, courselike_source_key, block_key, parent, expected): + """ + Test that derived_key returns the expected value. + """ + self.assertEqual(derived_key(courselike_source_key, block_key, parent), expected)