From fcb4c4d09818a75fead154fa554ded699fd2db1e Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Thu, 25 May 2023 17:45:11 +0500 Subject: [PATCH 1/5] fix: replacing exceptions (#31827) * fix: fixing test. * fix: updating exceptions. --- lms/djangoapps/bulk_email/tasks.py | 96 +++++++++---------- lms/djangoapps/bulk_email/tests/test_tasks.py | 64 +++++-------- 2 files changed, 68 insertions(+), 92 deletions(-) diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index 4d86edcc10..afad888fe0 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/lms/djangoapps/bulk_email/tasks.py @@ -10,21 +10,10 @@ import re import time from collections import Counter from datetime import datetime -from smtplib import SMTPConnectError, SMTPDataError, SMTPException, SMTPServerDisconnected, SMTPSenderRefused +from smtplib import SMTPConnectError, SMTPDataError, SMTPException, SMTPSenderRefused, SMTPServerDisconnected from time import sleep -from boto.exception import AWSConnectionError -from boto.ses.exceptions import ( - SESAddressBlacklistedError, - SESAddressNotVerifiedError, - SESDailyQuotaExceededError, - SESDomainEndsWithDotError, - SESDomainNotConfirmedError, - SESIdentityNotVerifiedError, - SESIllegalAddressError, - SESLocalAddressCharacterError, - SESMaxSendingRateExceededError -) +from botocore.exceptions import ClientError, EndpointConnectionError from celery import current_task, shared_task from celery.exceptions import RetryTaskError from celery.states import FAILURE, RETRY, SUCCESS @@ -34,8 +23,8 @@ from django.core.mail import get_connection from django.core.mail.message import forbid_multi_line_headers from django.urls import reverse from django.utils import timezone -from django.utils.translation import override as override_language from django.utils.translation import gettext as _ +from django.utils.translation import override as override_language from edx_django_utils.monitoring import set_code_owner_attribute from markupsafe import escape @@ -43,15 +32,12 @@ from common.djangoapps.util.date_utils import get_default_time_display from common.djangoapps.util.string_utils import _has_non_ascii_characters from lms.djangoapps.branding.api import get_logo_url_for_email from lms.djangoapps.bulk_email.api import get_unsubscribed_link -from lms.djangoapps.bulk_email.messages import ( - DjangoEmail, - ACEEmail, -) +from lms.djangoapps.bulk_email.messages import ACEEmail, DjangoEmail +from lms.djangoapps.bulk_email.models import CourseEmail, Optout from lms.djangoapps.bulk_email.toggles import ( is_bulk_email_edx_ace_enabled, - is_email_use_course_id_from_for_bulk_enabled, + is_email_use_course_id_from_for_bulk_enabled ) -from lms.djangoapps.bulk_email.models import CourseEmail, Optout from lms.djangoapps.courseware.courses import get_course from lms.djangoapps.instructor_task.models import InstructorTask from lms.djangoapps.instructor_task.subtasks import ( @@ -66,13 +52,11 @@ from openedx.core.lib.courses import course_image_url log = logging.getLogger('edx.celery.task') + # Errors that an individual email is failing to be sent, and should just # be treated as a fail. SINGLE_EMAIL_FAILURE_ERRORS = ( - SESAddressBlacklistedError, # Recipient's email address has been temporarily blacklisted. - SESDomainEndsWithDotError, # Recipient's email address' domain ends with a period/dot. - SESIllegalAddressError, # Raised when an illegal address is encountered. - SESLocalAddressCharacterError, # An address contained a control or whitespace character. + ClientError ) # Exceptions that, if caught, should cause the task to be re-tried. @@ -80,7 +64,7 @@ SINGLE_EMAIL_FAILURE_ERRORS = ( LIMITED_RETRY_ERRORS = ( SMTPConnectError, SMTPServerDisconnected, - AWSConnectionError, + EndpointConnectionError, ) # Errors that indicate that a mailing task should be retried without limit. @@ -91,20 +75,17 @@ LIMITED_RETRY_ERRORS = ( # Those not in this range (i.e. in the 5xx range) are treated as hard failures # and thus like SINGLE_EMAIL_FAILURE_ERRORS. INFINITE_RETRY_ERRORS = ( - SESMaxSendingRateExceededError, # Your account's requests/second limit has been exceeded. SMTPDataError, SMTPSenderRefused, + ClientError ) # Errors that are known to indicate an inability to send any more emails, # and should therefore not be retried. For example, exceeding a quota for emails. # Also, any SMTP errors that are not explicitly enumerated above. BULK_EMAIL_FAILURE_ERRORS = ( - SESAddressNotVerifiedError, # Raised when a "Reply-To" address has not been validated in SES yet. - SESIdentityNotVerifiedError, # Raised when an identity has not been verified in SES yet. - SESDomainNotConfirmedError, # Raised when domain ownership is not confirmed for DKIM. - SESDailyQuotaExceededError, # 24-hour allotment of outbound email has been exceeded. - SMTPException, + ClientError, + SMTPException ) @@ -588,13 +569,16 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas except SINGLE_EMAIL_FAILURE_ERRORS as exc: # This will fall through and not retry the message. - total_recipients_failed += 1 - log.exception( - f"BulkEmail ==> Status: Failed(SINGLE_EMAIL_FAILURE_ERRORS), Task: {parent_task_id}, SubTask: " - f"{task_id}, EmailId: {email_id}, Recipient num: {recipient_num}/{total_recipients}, Recipient " - f"UserId: {current_recipient['pk']}" - ) - subtask_status.increment(failed=1) + if exc.response['Error']['Code'] in ['MessageRejected', 'MailFromDomainNotVerified', 'MailFromDomainNotVerifiedException', 'FromEmailAddressNotVerifiedException']: # lint-amnesty, pylint: disable=line-too-long + total_recipients_failed += 1 + log.exception( + f"BulkEmail ==> Status: Failed(SINGLE_EMAIL_FAILURE_ERRORS), Task: {parent_task_id}, SubTask: " + f"{task_id}, EmailId: {email_id}, Recipient num: {recipient_num}/{total_recipients}, Recipient " + f"UserId: {current_recipient['pk']}" + ) + subtask_status.increment(failed=1) + else: + raise exc else: total_recipients_successful += 1 @@ -631,10 +615,13 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas except INFINITE_RETRY_ERRORS as exc: # Increment the "retried_nomax" counter, update other counters with progress to date, # and set the state to RETRY: - subtask_status.increment(retried_nomax=1, state=RETRY) - return _submit_for_retry( - entry_id, email_id, to_list, global_email_context, exc, subtask_status, skip_retry_max=True - ) + if isinstance(exc, (SMTPDataError, SMTPSenderRefused)) or exc.response['Error']['Code'] in ['LimitExceededException']: # lint-amnesty, pylint: disable=line-too-long + subtask_status.increment(retried_nomax=1, state=RETRY) + return _submit_for_retry( + entry_id, email_id, to_list, global_email_context, exc, subtask_status, skip_retry_max=True + ) + else: + raise exc except LIMITED_RETRY_ERRORS as exc: # Errors caught here cause the email to be retried. The entire task is actually retried @@ -642,21 +629,28 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas # Errors caught are those that indicate a temporary condition that might succeed on retry. # Increment the "retried_withmax" counter, update other counters with progress to date, # and set the state to RETRY: + subtask_status.increment(retried_withmax=1, state=RETRY) return _submit_for_retry( entry_id, email_id, to_list, global_email_context, exc, subtask_status, skip_retry_max=False ) except BULK_EMAIL_FAILURE_ERRORS as exc: - num_pending = len(to_list) - log.exception( - f"Task {task_id}: email with id {email_id} caused send_course_email task to fail with 'fatal' exception. " - f"{num_pending} emails unsent." - ) - # Update counters with progress to date, counting unsent emails as failures, - # and set the state to FAILURE: - subtask_status.increment(failed=num_pending, state=FAILURE) - return subtask_status, exc + if isinstance(exc, SMTPException) or exc.response['Error']['Code'] in [ + 'AccountSendingPausedException', 'MailFromDomainNotVerifiedException', 'LimitExceededException' + ]: + num_pending = len(to_list) + log.exception( + f"Task {task_id}: email with id {email_id} caused send_course_email " + f"task to fail with 'fatal' exception. " + f"{num_pending} emails unsent." + ) + # Update counters with progress to date, counting unsent emails as failures, + # and set the state to FAILURE: + subtask_status.increment(failed=num_pending, state=FAILURE) + return subtask_status, exc + else: + raise exc except Exception as exc: # pylint: disable=broad-except # Errors caught here cause the email to be retried. The entire task is actually retried diff --git a/lms/djangoapps/bulk_email/tests/test_tasks.py b/lms/djangoapps/bulk_email/tests/test_tasks.py index e73db046aa..96c48e0d9e 100644 --- a/lms/djangoapps/bulk_email/tests/test_tasks.py +++ b/lms/djangoapps/bulk_email/tests/test_tasks.py @@ -7,27 +7,23 @@ paths actually work. """ -from datetime import datetime -from dateutil.relativedelta import relativedelta import json # lint-amnesty, pylint: disable=wrong-import-order +from datetime import datetime from itertools import chain, cycle, repeat # lint-amnesty, pylint: disable=wrong-import-order -from smtplib import SMTPAuthenticationError, SMTPConnectError, SMTPDataError, SMTPServerDisconnected, SMTPSenderRefused # lint-amnesty, pylint: disable=wrong-import-order +from smtplib import ( # lint-amnesty, pylint: disable=wrong-import-order + SMTPAuthenticationError, + SMTPConnectError, + SMTPDataError, + SMTPSenderRefused, + SMTPServerDisconnected +) from unittest.mock import Mock, patch # lint-amnesty, pylint: disable=wrong-import-order from uuid import uuid4 # lint-amnesty, pylint: disable=wrong-import-order + import pytest -from boto.exception import AWSConnectionError -from boto.ses.exceptions import ( - SESAddressBlacklistedError, - SESAddressNotVerifiedError, - SESDailyQuotaExceededError, - SESDomainEndsWithDotError, - SESDomainNotConfirmedError, - SESIdentityNotVerifiedError, - SESIllegalAddressError, - SESLocalAddressCharacterError, - SESMaxSendingRateExceededError -) +from botocore.exceptions import ClientError, EndpointConnectionError from celery.states import FAILURE, SUCCESS +from dateutil.relativedelta import relativedelta from django.conf import settings from django.core.management import call_command from django.test.utils import override_settings @@ -268,19 +264,22 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase): def test_ses_blacklisted_user(self): # Test that celery handles permanent SMTPDataErrors by failing and not retrying. - self._test_email_address_failures(SESAddressBlacklistedError(554, "Email address is blacklisted")) + + operation_name = '' + parsed_response = {'Error': {'Code': 'MessageRejected', 'Message': 'Error Uploading'}} + self._test_email_address_failures(ClientError(parsed_response, operation_name)) def test_ses_illegal_address(self): # Test that celery handles permanent SMTPDataErrors by failing and not retrying. - self._test_email_address_failures(SESIllegalAddressError(554, "Email address is illegal")) - - def test_ses_local_address_character_error(self): - # Test that celery handles permanent SMTPDataErrors by failing and not retrying. - self._test_email_address_failures(SESLocalAddressCharacterError(554, "Email address contains a bad character")) + operation_name = '' + parsed_response = {'Error': {'Code': 'MailFromDomainNotVerifiedException', 'Message': 'Error Uploading'}} + self._test_email_address_failures(ClientError(parsed_response, operation_name)) def test_ses_domain_ends_with_dot(self): # Test that celery handles permanent SMTPDataErrors by failing and not retrying. - self._test_email_address_failures(SESDomainEndsWithDotError(554, "Email address ends with a dot")) + operation_name = '' + parsed_response = {'Error': {'Code': 'MailFromDomainNotVerifiedException', 'Message': 'invalid domain'}} + self._test_email_address_failures(ClientError(parsed_response, operation_name)) def test_bulk_email_skip_with_non_ascii_emails(self): """ @@ -367,12 +366,12 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase): def test_retry_after_aws_connect_error(self): self._test_retry_after_limited_retry_error( - AWSConnectionError("Unable to provide secure connection through proxy") + EndpointConnectionError(endpoint_url="Could not connect to the endpoint URL:") ) def test_max_retry_after_aws_connect_error(self): self._test_max_retry_limit_causes_failure( - AWSConnectionError("Unable to provide secure connection through proxy") + EndpointConnectionError(endpoint_url="Could not connect to the endpoint URL:") ) def test_retry_after_general_error(self): @@ -416,11 +415,6 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase): SMTPSenderRefused(421, "Throttling: Sending rate exceeded", self.instructor.email) ) - def test_retry_after_ses_throttling_error(self): - self._test_retry_after_unlimited_retry_error( - SESMaxSendingRateExceededError(455, "Throttling: Sending rate exceeded") - ) - def _test_immediate_failure(self, exception): """Test that celery can hit a maximum number of retries.""" # Doesn't really matter how many recipients, since we expect @@ -444,18 +438,6 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase): def test_failure_on_unhandled_smtp(self): self._test_immediate_failure(SMTPAuthenticationError(403, "That password doesn't work!")) - def test_failure_on_ses_quota_exceeded(self): - self._test_immediate_failure(SESDailyQuotaExceededError(403, "You're done for the day!")) - - def test_failure_on_ses_address_not_verified(self): - self._test_immediate_failure(SESAddressNotVerifiedError(403, "Who *are* you?")) - - def test_failure_on_ses_identity_not_verified(self): - self._test_immediate_failure(SESIdentityNotVerifiedError(403, "May I please see an ID!")) - - def test_failure_on_ses_domain_not_confirmed(self): - self._test_immediate_failure(SESDomainNotConfirmedError(403, "You're out of bounds!")) - def test_bulk_emails_with_unicode_course_image_name(self): # Test bulk email with unicode characters in course image name course_image = '在淡水測試.jpg' From aa7370c773dcd96850fbda8e4f6ad40950c10547 Mon Sep 17 00:00:00 2001 From: Fox Piacenti Date: Thu, 25 May 2023 08:58:28 -0500 Subject: [PATCH 2/5] refactor: Duplicate and update primitives made available. This makes a couple of changes to the xblock handler in the CMS. These changes add a handful of utility functions and modify the existing ones to make reuse of existing blocks easier. With these changes, it is possible to copy an entire section from one course to another, and then later refresh that section, and all of its children, without destroying the blocks next to it. The existing _duplicate_block function was modified to have a shallow keyword to avoid copying children, and the update_from_source function was added to make it easy to copy attributes over from one block to another. These functions can be used alongside copy_from_template in the modulestore to copy over blocks and their children without requiring them to be within any particular container (other than a library or course root)-- thus allowing library-like inclusion without the library content block. This is especially useful for cases like copying sections rather than unit content. --- .../contentstore/tests/test_libraries.py | 8 +- cms/djangoapps/contentstore/utils.py | 214 +++++++++++++++++- cms/djangoapps/contentstore/views/block.py | 151 +----------- .../contentstore/views/component.py | 5 +- .../contentstore/views/tests/test_block.py | 134 ++++++++++- xmodule/modulestore/split_mongo/split.py | 17 +- xmodule/modulestore/store_utilities.py | 32 ++- .../modulestore/tests/test_store_utilities.py | 46 +++- 8 files changed, 435 insertions(+), 172 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py index 019f67a594..eb9b109cdd 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -15,8 +15,8 @@ from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory from xmodule.x_module import STUDIO_VIEW 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.utils import reverse_library_url, reverse_url, \ + reverse_usage_url, 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 @@ class TestOverrides(LibraryTestCase): 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 @@ class TestOverrides(LibraryTestCase): # 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/utils.py b/cms/djangoapps/contentstore/utils.py index 14f86917b4..ed1d1e5df8 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -5,20 +5,28 @@ Common utility functions useful throughout the contentstore from collections import defaultdict import logging from contextlib import contextmanager -from datetime import datetime +from datetime import datetime, timezone +from uuid import uuid4 from django.conf import settings from django.urls import reverse from django.utils import translation from django.utils.translation import gettext as _ +from lti_consumer.models import CourseAllowPIISharingInLTIFlag from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locator import LibraryLocator +from openedx_events.content_authoring.data import DuplicatedXBlockData +from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED from pytz import UTC +from xblock.fields import Scope from cms.djangoapps.contentstore.toggles import exam_setting_view_enabled +from common.djangoapps.edxmako.services import MakoService from common.djangoapps.student import auth +from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole +from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from openedx.core.djangoapps.course_apps.toggles import proctoring_settings_modal_view_enabled from openedx.core.djangoapps.discussions.config.waffle import ENABLE_PAGES_AND_RESOURCES_MICROFRONTEND from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration @@ -29,8 +37,6 @@ from openedx.core.djangoapps.site_configuration.models import SiteConfiguration from openedx.features.content_type_gating.models import ContentTypeGatingConfig from openedx.features.content_type_gating.partitions import CONTENT_TYPE_GATING_SCHEME from cms.djangoapps.contentstore.toggles import ( - use_new_text_editor, - use_new_video_editor, use_new_advanced_settings_page, use_new_course_outline_page, use_new_export_page, @@ -44,10 +50,13 @@ from cms.djangoapps.contentstore.toggles import ( use_new_updates_page, use_new_video_uploads_page, ) +from cms.djangoapps.contentstore.toggles import use_new_text_editor, use_new_video_editor +from xmodule.library_tools import LibraryToolsService from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order from xmodule.partitions.partitions_service import get_all_partitions_for_course # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.services import SettingsService, ConfigurationService, TeamsConfigurationService log = logging.getLogger(__name__) @@ -935,3 +944,202 @@ def update_course_discussions_settings(course_key): course = store.get_course(course_key) course.discussions_settings['provider_type'] = provider store.update_item(course, course.published_by) + + +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. You can + optionally specify what usage key the new duplicate block will use via dest_usage_key. + + If shallow is True, does not copy children. Otherwise, this function calls itself + recursively, and will set the is_child flag to True when dealing with recursed child + blocks. + """ + store = modulestore() + with store.bulk_operations(duplicate_source_usage_key.course_key): + source_item = store.get_item(duplicate_source_usage_key) + if not dest_usage_key: + # Change the blockID to be unique. + dest_usage_key = source_item.location.replace(name=uuid4().hex) + + category = dest_usage_key.block_type + + 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, + dest_usage_key.course_key, + dest_usage_key.block_type, + block_id=dest_usage_key.block_id, + definition_data=source_item.get_explicitly_set_fields_by_scope(Scope.content), + metadata=duplicate_metadata, + runtime=source_item.runtime, + asides=asides_to_create + ) + + children_handled = False + + if hasattr(dest_block, 'studio_post_duplicate'): + # Allow an XBlock to do anything fancy it may need to when duplicated from another block. + # These blocks may handle their own children or parenting if needed. Let them return booleans to + # let us know if we need to handle these or not. + load_services_for_studio(dest_block.runtime, user) + children_handled = dest_block.studio_post_duplicate(store, source_item) + + # 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 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) + 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) + + # pylint: disable=protected-access + if 'detached' not in source_item.runtime.load_block_type(category)._class_tags: + parent = store.get_item(parent_usage_key) + # If source was already a child of the parent, add duplicate immediately afterward. + # Otherwise, add child to end. + if source_item.location in parent.children: + source_index = parent.children.index(source_item.location) + parent.children.insert(source_index + 1, dest_block.location) + else: + parent.children.append(dest_block.location) + store.update_item(parent, user.id) + + # .. event_implemented_name: XBLOCK_DUPLICATED + XBLOCK_DUPLICATED.send_event( + time=datetime.now(timezone.utc), + xblock_info=DuplicatedXBlockData( + usage_key=dest_block.location, + block_type=dest_block.location.block_type, + source_usage_key=duplicate_source_usage_key, + ) + ) + + return dest_block.location + + +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. + + This function is useful when a block, originally copied from a source block, drifts + and needs to be updated to match the original. + + The modulestore function copy_from_template will copy a block's children recursively, + replacing the target block's children. It does not, however, update any of the target + block's settings. copy_from_template, then, is useful for cases like the Library + Content Block, where the children are the same across all instances, but the settings + may differ. + + By contrast, for cases where we're copying a block that has drifted from its source, + we need to update the target block's settings, but we don't want to replace its children, + or, at least, not only replace its children. update_from_source is useful for these cases. + + This function is meant to be imported by pluggable django apps looking to manage duplicated + sections of a course. It is placed here for lack of a more appropriate location, since this + code has not yet been brought up to the standards in OEP-45. + """ + 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 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 load_services_for_studio(runtime, user): + """ + Function to set some required services used for XBlock edits and studio_view. + (i.e. whenever we're not loading _prepare_runtime_for_preview.) This is required to make information + about the current user (especially permissions) available via services as needed. + """ + services = { + "user": DjangoXBlockUserService(user), + "studio_user_permissions": StudioPermissionsService(user), + "mako": MakoService(), + "settings": SettingsService(), + "lti-configuration": ConfigurationService(CourseAllowPIISharingInLTIFlag), + "teams_configuration": TeamsConfigurationService(), + "library_tools": LibraryToolsService(modulestore(), user.id) + } + + runtime._services.update(services) # lint-amnesty, pylint: disable=protected-access + + +class StudioPermissionsService: + """ + Service that can provide information about a user's permissions. + + Deprecated. To be replaced by a more general authorization service. + + Only used by LibraryContentBlock (and library_tools.py). + """ + def __init__(self, user): + self._user = user + + def can_read(self, course_key): + """ Does the user have read access to the given course/library? """ + return has_studio_read_access(self._user, course_key) + + def can_write(self, course_key): + """ Does the user have read access to the given course/library? """ + return has_studio_write_access(self._user, course_key) diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index 2cadea08ea..a35230f3b8 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -4,19 +4,15 @@ import logging from collections import OrderedDict from datetime import datetime from functools import partial -from uuid import uuid4 from django.conf import settings from django.contrib.auth.decorators import login_required from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.core.exceptions import PermissionDenied from django.http import Http404, HttpResponse, HttpResponseBadRequest -from django.utils.timezone import timezone from django.utils.translation import gettext as _ from django.views.decorators.http import require_http_methods from edx_django_utils.plugins import pluggable_override -from openedx_events.content_authoring.data import DuplicatedXBlockData -from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED from edx_proctoring.api import ( does_backend_support_onboarding, get_exam_by_content_id, @@ -24,7 +20,6 @@ from edx_proctoring.api import ( ) from edx_proctoring.exceptions import ProctoredExamNotFoundException from help_tokens.core import HelpUrlExpert -from lti_consumer.models import CourseAllowPIISharingInLTIFlag from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryUsageLocator from pytz import UTC @@ -35,13 +30,11 @@ from xblock.fields import Scope from cms.djangoapps.contentstore.config.waffle import SHOW_REVIEW_RULES_FLAG 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 from common.djangoapps.edxmako.shortcuts import render_to_string from common.djangoapps.static_replace import replace_static_urls from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access from common.djangoapps.util.date_utils import get_default_time_display from common.djangoapps.util.json_request import JsonResponse, expect_json -from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from openedx.core.djangoapps.bookmarks import api as bookmarks_api from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE @@ -49,13 +42,11 @@ from openedx.core.lib.gating import api as gating_api from openedx.core.lib.xblock_utils import hash_resource, request_token, wrap_xblock, wrap_xblock_aside from openedx.core.toggles import ENTRANCE_EXAMS from xmodule.course_block import DEFAULT_START_DATE # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.library_tools import LibraryToolsService # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore import EdxJSONEncoder, ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.inheritance import own_metadata # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.services import ConfigurationService, SettingsService, TeamsConfigurationService # lint-amnesty, pylint: disable=wrong-import-order from xmodule.tabs import CourseTabList # lint-amnesty, pylint: disable=wrong-import-order from xmodule.x_module import AUTHOR_VIEW, PREVIEW_VIEWS, STUDENT_VIEW, STUDIO_VIEW # lint-amnesty, pylint: disable=wrong-import-order @@ -68,7 +59,7 @@ from ..utils import ( get_visibility_partition_info, has_children_visible_to_specific_partition_groups, is_currently_visible_to_students, - is_self_paced + is_self_paced, duplicate_block, load_services_for_studio ) from .helpers import ( create_xblock, @@ -245,11 +236,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), @@ -277,45 +268,6 @@ def xblock_handler(request, usage_key_string=None): ) -class StudioPermissionsService: - """ - Service that can provide information about a user's permissions. - - Deprecated. To be replaced by a more general authorization service. - - Only used by LibraryContentBlock (and library_tools.py). - """ - def __init__(self, user): - self._user = user - - def can_read(self, course_key): - """ Does the user have read access to the given course/library? """ - return has_studio_read_access(self._user, course_key) - - def can_write(self, course_key): - """ Does the user have read access to the given course/library? """ - return has_studio_write_access(self._user, course_key) - - -def load_services_for_studio(runtime, user): - """ - Function to set some required services used for XBlock edits and studio_view. - (i.e. whenever we're not loading _prepare_runtime_for_preview.) This is required to make information - about the current user (especially permissions) available via services as needed. - """ - services = { - "user": DjangoXBlockUserService(user), - "studio_user_permissions": StudioPermissionsService(user), - "mako": MakoService(), - "settings": SettingsService(), - "lti-configuration": ConfigurationService(CourseAllowPIISharingInLTIFlag), - "teams_configuration": TeamsConfigurationService(), - "library_tools": LibraryToolsService(modulestore(), user.id) - } - - runtime._services.update(services) # lint-amnesty, pylint: disable=protected-access - - @require_http_methods("GET") @login_required @expect_json @@ -880,103 +832,6 @@ 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): - """ - 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 - - # 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) - - dest_block = store.create_item( - user.id, - dest_usage_key.course_key, - dest_usage_key.block_type, - block_id=dest_usage_key.block_id, - definition_data=source_item.get_explicitly_set_fields_by_scope(Scope.content), - metadata=duplicate_metadata, - runtime=source_item.runtime, - asides=asides_to_create - ) - - children_handled = False - - if hasattr(dest_block, 'studio_post_duplicate'): - # Allow an XBlock to do anything fancy it may need to when duplicated from another block. - # These blocks may handle their own children or parenting if needed. Let them return booleans to - # let us know if we need to handle these or not. - load_services_for_studio(dest_block.runtime, user) - children_handled = dest_block.studio_post_duplicate(store, source_item) - - # 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: - 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) - 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) - - # pylint: disable=protected-access - if 'detached' not in source_item.runtime.load_block_type(category)._class_tags: - parent = store.get_item(parent_usage_key) - # If source was already a child of the parent, add duplicate immediately afterward. - # Otherwise, add child to end. - if source_item.location in parent.children: - source_index = parent.children.index(source_item.location) - parent.children.insert(source_index + 1, dest_block.location) - else: - parent.children.append(dest_block.location) - store.update_item(parent, user.id) - - # .. event_implemented_name: XBLOCK_DUPLICATED - XBLOCK_DUPLICATED.send_event( - time=datetime.now(timezone.utc), - xblock_info=DuplicatedXBlockData( - usage_key=dest_block.location, - block_type=dest_block.location.block_type, - source_usage_key=duplicate_source_usage_key, - ) - ) - - return dest_block.location - - @login_required @expect_json def delete_item(request, usage_key): diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 60d6f68c9a..8aadd41869 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -35,9 +35,10 @@ except ImportError: from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order -from ..utils import get_lms_link_for_item, get_sibling_urls, reverse_course_url +from ..utils import get_lms_link_for_item, get_sibling_urls, reverse_course_url, \ + load_services_for_studio from .helpers import get_parent_xblock, is_unit, xblock_type_display_name -from .block import add_container_page_publishing_info, create_xblock_info, load_services_for_studio +from .block import add_container_page_publishing_info, create_xblock_info __all__ = [ 'container_handler', diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index 841e93c656..bb827d361c 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -48,7 +48,7 @@ from xmodule.partitions.tests.test_partitions import MockPartitionService from xmodule.x_module import STUDENT_VIEW, STUDIO_VIEW from cms.djangoapps.contentstore.tests.utils import CourseTestCase -from cms.djangoapps.contentstore.utils import reverse_course_url, reverse_usage_url +from cms.djangoapps.contentstore.utils import reverse_course_url, reverse_usage_url, duplicate_block, update_from_source from cms.djangoapps.contentstore.views import block as item_module from common.djangoapps.student.tests.factories import StaffFactory, UserFactory from common.djangoapps.xblock_django.models import ( @@ -787,6 +787,30 @@ class TestDuplicateItem(ItemTest, DuplicateHelper, OpenEdxEventsTestMixin): # 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 duplicate_block(..., shallow=True) can duplicate a block but ignores its children. + """ + 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): @@ -3495,3 +3519,111 @@ class TestXBlockPublishingInfo(ItemTest): # 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) + + +@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 replaces all settings on the block. + """ + 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/xmodule/modulestore/split_mongo/split.py b/xmodule/modulestore/split_mongo/split.py index e76f6c8120..f72f950a54 100644 --- a/xmodule/modulestore/split_mongo/split.py +++ b/xmodule/modulestore/split_mongo/split.py @@ -57,7 +57,6 @@ Representation: import copy import datetime -import hashlib import logging from collections import defaultdict from importlib import import_module @@ -102,7 +101,7 @@ from xmodule.modulestore.exceptions import ( ) 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 @@ -2369,6 +2368,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): time this method is called for the same source block and dest_usage, the same resulting block id will be generated. + Note also that this function does not override any of the attributes on the destination + block-- it only replaces the destination block's children. + :param source_keys: a list of BlockUsageLocators. Order is preserved. :param dest_usage: The BlockUsageLocator that will become the parent of an inherited copy @@ -2442,7 +2444,6 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): 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] @@ -2450,15 +2451,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): 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 45082ff43e..ced5c9728e 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 collections import namedtuple 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,31 @@ 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, dest_parent: BlockKey): + """ + 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. + + This is used by the copy_from_template function of the modulestore, and can be used by + pluggable django apps that need to copy blocks from one course to another in an + idempotent way. In the future, this should be created into a proper API function + in the spirit of OEP-49. + """ + 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, + dest_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 1c1c86f4fc..a0b5cfcbdb 100644 --- a/xmodule/modulestore/tests/test_store_utilities.py +++ b/xmodule/modulestore/tests/test_store_utilities.py @@ -4,11 +4,15 @@ Tests for store_utilities.py 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 @@ class TestUtils(unittest.TestCase): 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) From 0f847df73adfa3dd54d98ec509d91199ee3f6e37 Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Thu, 25 May 2023 13:30:39 -0400 Subject: [PATCH 3/5] refactor: define resource paths (not contents) on XModule classes (#32286) For the XBlocks types that use legacy XModule-style assets (specifically, those that inherit from `HTMLSnippet`), this is small refactor that brings them a bit closer to being like standard XBlocks. Given these class attributes: class SomeXModuleLikeBlock(..., HTMLSnippet, ...): ... studio_view_css = { ... } preview_view_css = { ... } studio_view_js = { ... } preview_view_js = { ... } ... we make it so their values are *paths to the resources* rather than *the actual content of the resources*. This is a no-op change, but it'll enable future XModule asset refactorings which require us to operate on asset paths rather than contents. Part of: https://github.com/openedx/edx-platform/issues/32292 --- xmodule/annotatable_block.py | 20 ++++++++++---------- xmodule/capa_block.py | 24 ++++++++++++------------ xmodule/conditional_block.py | 14 +++++++------- xmodule/html_block.py | 24 ++++++++++++------------ xmodule/library_content_block.py | 8 ++++---- xmodule/lti_block.py | 12 ++++++------ xmodule/poll_block.py | 14 +++++++------- xmodule/seq_block.py | 10 +++++----- xmodule/split_test_block.py | 8 ++++---- xmodule/static_content.py | 28 +++++++++++++++++----------- xmodule/template_block.py | 10 +++++----- xmodule/word_cloud_block.py | 12 ++++++------ 12 files changed, 95 insertions(+), 89 deletions(-) diff --git a/xmodule/annotatable_block.py b/xmodule/annotatable_block.py index 774ad90097..9ec6bf21da 100644 --- a/xmodule/annotatable_block.py +++ b/xmodule/annotatable_block.py @@ -4,7 +4,7 @@ import logging import textwrap from lxml import etree -from pkg_resources import resource_string +from pkg_resources import resource_filename from web_fragments.fragment import Fragment from xblock.core import XBlock from xblock.fields import Scope, String @@ -75,28 +75,28 @@ class AnnotatableBlock( preview_view_js = { 'js': [ - resource_string(__name__, 'js/src/html/display.js'), - resource_string(__name__, 'js/src/annotatable/display.js'), - resource_string(__name__, 'js/src/javascript_loader.js'), - resource_string(__name__, 'js/src/collapsible.js'), + resource_filename(__name__, 'js/src/html/display.js'), + resource_filename(__name__, 'js/src/annotatable/display.js'), + resource_filename(__name__, 'js/src/javascript_loader.js'), + resource_filename(__name__, 'js/src/collapsible.js'), ], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), } preview_view_css = { 'scss': [ - resource_string(__name__, 'css/annotatable/display.scss'), + resource_filename(__name__, 'css/annotatable/display.scss'), ], } studio_view_js = { 'js': [ - resource_string(__name__, 'js/src/raw/edit/xml.js'), + resource_filename(__name__, 'js/src/raw/edit/xml.js'), ], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), } studio_view_css = { 'scss': [ - resource_string(__name__, 'css/codemirror/codemirror.scss'), + resource_filename(__name__, 'css/codemirror/codemirror.scss'), ], } studio_js_module_name = "XMLEditingDescriptor" diff --git a/xmodule/capa_block.py b/xmodule/capa_block.py index 3c02053c0e..2b2ec80825 100644 --- a/xmodule/capa_block.py +++ b/xmodule/capa_block.py @@ -19,7 +19,7 @@ from django.core.exceptions import ImproperlyConfigured from django.utils.encoding import smart_str from django.utils.functional import cached_property from lxml import etree -from pkg_resources import resource_string +from pkg_resources import resource_filename from pytz import utc from web_fragments.fragment import Fragment from xblock.core import XBlock @@ -168,32 +168,32 @@ class ProblemBlock( preview_view_js = { 'js': [ - resource_string(__name__, 'js/src/javascript_loader.js'), - resource_string(__name__, 'js/src/capa/display.js'), - resource_string(__name__, 'js/src/collapsible.js'), - resource_string(__name__, 'js/src/capa/imageinput.js'), - resource_string(__name__, 'js/src/capa/schematic.js'), + resource_filename(__name__, 'js/src/javascript_loader.js'), + resource_filename(__name__, 'js/src/capa/display.js'), + resource_filename(__name__, 'js/src/collapsible.js'), + resource_filename(__name__, 'js/src/capa/imageinput.js'), + resource_filename(__name__, 'js/src/capa/schematic.js'), ], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js') + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js') } preview_view_css = { 'scss': [ - resource_string(__name__, 'css/capa/display.scss'), + resource_filename(__name__, 'css/capa/display.scss'), ], } studio_view_js = { 'js': [ - resource_string(__name__, 'js/src/problem/edit.js'), + resource_filename(__name__, 'js/src/problem/edit.js'), ], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), } studio_view_css = { 'scss': [ - resource_string(__name__, 'css/editor/edit.scss'), - resource_string(__name__, 'css/problem/edit.scss'), + resource_filename(__name__, 'css/editor/edit.scss'), + resource_filename(__name__, 'css/problem/edit.scss'), ] } diff --git a/xmodule/conditional_block.py b/xmodule/conditional_block.py index 91e7af2c15..7fe545d1a3 100644 --- a/xmodule/conditional_block.py +++ b/xmodule/conditional_block.py @@ -9,7 +9,7 @@ import logging from lazy import lazy from lxml import etree from opaque_keys.edx.locator import BlockUsageLocator -from pkg_resources import resource_string +from pkg_resources import resource_filename from web_fragments.fragment import Fragment from xblock.core import XBlock from xblock.fields import ReferenceList, Scope, String @@ -148,11 +148,11 @@ class ConditionalBlock( preview_view_js = { 'js': [ - resource_string(__name__, 'js/src/conditional/display.js'), - resource_string(__name__, 'js/src/javascript_loader.js'), - resource_string(__name__, 'js/src/collapsible.js'), + resource_filename(__name__, 'js/src/conditional/display.js'), + resource_filename(__name__, 'js/src/javascript_loader.js'), + resource_filename(__name__, 'js/src/collapsible.js'), ], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), } preview_view_css = { 'scss': [], @@ -161,8 +161,8 @@ class ConditionalBlock( mako_template = 'widgets/metadata-edit.html' studio_js_module_name = 'SequenceDescriptor' studio_view_js = { - 'js': [resource_string(__name__, 'js/src/sequence/edit.js')], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + 'js': [resource_filename(__name__, 'js/src/sequence/edit.js')], + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), } studio_view_css = { 'scss': [], diff --git a/xmodule/html_block.py b/xmodule/html_block.py index ed5d84e395..48786c2c56 100644 --- a/xmodule/html_block.py +++ b/xmodule/html_block.py @@ -8,7 +8,7 @@ import sys import textwrap from datetime import datetime -from pkg_resources import resource_string +from pkg_resources import resource_filename from django.conf import settings from fs.errors import ResourceNotFound @@ -144,15 +144,15 @@ class HtmlBlockMixin( # lint-amnesty, pylint: disable=abstract-method preview_view_js = { 'js': [ - resource_string(__name__, 'js/src/html/display.js'), - resource_string(__name__, 'js/src/javascript_loader.js'), - resource_string(__name__, 'js/src/collapsible.js'), - resource_string(__name__, 'js/src/html/imageModal.js'), - resource_string(__name__, 'js/common_static/js/vendor/draggabilly.js'), + resource_filename(__name__, 'js/src/html/display.js'), + resource_filename(__name__, 'js/src/javascript_loader.js'), + resource_filename(__name__, 'js/src/collapsible.js'), + resource_filename(__name__, 'js/src/html/imageModal.js'), + resource_filename(__name__, 'js/common_static/js/vendor/draggabilly.js'), ], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), } - preview_view_css = {'scss': [resource_string(__name__, 'css/html/display.scss')]} + preview_view_css = {'scss': [resource_filename(__name__, 'css/html/display.scss')]} uses_xmodule_styles_setup = True @@ -164,14 +164,14 @@ class HtmlBlockMixin( # lint-amnesty, pylint: disable=abstract-method studio_view_js = { 'js': [ - resource_string(__name__, 'js/src/html/edit.js') + resource_filename(__name__, 'js/src/html/edit.js') ], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), } studio_view_css = { 'scss': [ - resource_string(__name__, 'css/editor/edit.scss'), - resource_string(__name__, 'css/html/edit.scss') + resource_filename(__name__, 'css/editor/edit.scss'), + resource_filename(__name__, 'css/html/edit.scss') ] } diff --git a/xmodule/library_content_block.py b/xmodule/library_content_block.py index 04abe5de6e..7193247f58 100644 --- a/xmodule/library_content_block.py +++ b/xmodule/library_content_block.py @@ -17,7 +17,7 @@ from lazy import lazy from lxml import etree from lxml.etree import XMLSyntaxError from opaque_keys.edx.locator import LibraryLocator -from pkg_resources import resource_string +from pkg_resources import resource_filename from web_fragments.fragment import Fragment from webob import Response from xblock.completable import XBlockCompletionMode @@ -97,7 +97,7 @@ class LibraryContentBlock( preview_view_js = { 'js': [], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), } preview_view_css = { 'scss': [], @@ -107,9 +107,9 @@ class LibraryContentBlock( studio_js_module_name = "VerticalDescriptor" studio_view_js = { 'js': [ - resource_string(__name__, 'js/src/vertical/edit.js'), + resource_filename(__name__, 'js/src/vertical/edit.js'), ], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), } studio_view_css = { 'scss': [], diff --git a/xmodule/lti_block.py b/xmodule/lti_block.py index 8bf60f1c44..76c03d73a6 100644 --- a/xmodule/lti_block.py +++ b/xmodule/lti_block.py @@ -68,7 +68,7 @@ import oauthlib.oauth1 from django.conf import settings from lxml import etree from oauthlib.oauth1.rfc5849 import signature -from pkg_resources import resource_string +from pkg_resources import resource_filename from pytz import UTC from webob import Response from web_fragments.fragment import Fragment @@ -374,13 +374,13 @@ class LTIBlock( preview_view_js = { 'js': [ - resource_string(__name__, 'js/src/lti/lti.js') + resource_filename(__name__, 'js/src/lti/lti.js') ], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), } preview_view_css = { 'scss': [ - resource_string(__name__, 'css/lti/lti.scss') + resource_filename(__name__, 'css/lti/lti.scss') ], } @@ -389,9 +389,9 @@ class LTIBlock( studio_js_module_name = 'MetadataOnlyEditingDescriptor' studio_view_js = { 'js': [ - resource_string(__name__, 'js/src/raw/edit/metadata-only.js') + resource_filename(__name__, 'js/src/raw/edit/metadata-only.js') ], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), } studio_view_css = { 'scss': [], diff --git a/xmodule/poll_block.py b/xmodule/poll_block.py index b29ed710e9..0ce6d38e1e 100644 --- a/xmodule/poll_block.py +++ b/xmodule/poll_block.py @@ -13,7 +13,7 @@ import logging from collections import OrderedDict from copy import deepcopy -from pkg_resources import resource_string +from pkg_resources import resource_filename from web_fragments.fragment import Fragment from lxml import etree @@ -86,15 +86,15 @@ class PollBlock( preview_view_js = { 'js': [ - resource_string(__name__, 'js/src/javascript_loader.js'), - resource_string(__name__, 'js/src/poll/poll.js'), - resource_string(__name__, 'js/src/poll/poll_main.js') + resource_filename(__name__, 'js/src/javascript_loader.js'), + resource_filename(__name__, 'js/src/poll/poll.js'), + resource_filename(__name__, 'js/src/poll/poll_main.js') ], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), } preview_view_css = { 'scss': [ - resource_string(__name__, 'css/poll/display.scss') + resource_filename(__name__, 'css/poll/display.scss') ], } @@ -102,7 +102,7 @@ class PollBlock( # the static_content command happy. studio_view_js = { 'js': [], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js') + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js') } studio_view_css = { diff --git a/xmodule/seq_block.py b/xmodule/seq_block.py index 0d8be411fd..250a69d83b 100644 --- a/xmodule/seq_block.py +++ b/xmodule/seq_block.py @@ -14,7 +14,7 @@ from django.conf import settings from lxml import etree from opaque_keys.edx.keys import UsageKey -from pkg_resources import resource_string +from pkg_resources import resource_filename from pytz import UTC from web_fragments.fragment import Fragment from xblock.completable import XBlockCompletionMode @@ -273,14 +273,14 @@ class SequenceBlock( preview_view_js = { 'js': [ - resource_string(__name__, 'js/src/sequence/display.js'), + resource_filename(__name__, 'js/src/sequence/display.js'), ], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js') + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js') } preview_view_css = { 'scss': [ - resource_string(__name__, 'css/sequence/display.scss'), + resource_filename(__name__, 'css/sequence/display.scss'), ], } @@ -288,7 +288,7 @@ class SequenceBlock( # the static_content command happy. studio_view_js = { 'js': [], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js') + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js') } studio_view_css = { diff --git a/xmodule/split_test_block.py b/xmodule/split_test_block.py index 7feceebfee..b638eb7a50 100644 --- a/xmodule/split_test_block.py +++ b/xmodule/split_test_block.py @@ -12,7 +12,7 @@ from uuid import uuid4 from django.utils.functional import cached_property from lxml import etree -from pkg_resources import resource_string +from pkg_resources import resource_filename from web_fragments.fragment import Fragment from webob import Response from xblock.core import XBlock @@ -160,7 +160,7 @@ class SplitTestBlock( # lint-amnesty, pylint: disable=abstract-method preview_view_js = { 'js': [], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), } preview_view_css = { 'scss': [], @@ -169,8 +169,8 @@ class SplitTestBlock( # lint-amnesty, pylint: disable=abstract-method mako_template = "widgets/metadata-only-edit.html" studio_js_module_name = 'SequenceDescriptor' studio_view_js = { - 'js': [resource_string(__name__, 'js/src/sequence/edit.js')], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + 'js': [resource_filename(__name__, 'js/src/sequence/edit.js')], + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), } studio_view_css = { 'scss': [], diff --git a/xmodule/static_content.py b/xmodule/static_content.py index 2447347da9..58ffcb2de1 100755 --- a/xmodule/static_content.py +++ b/xmodule/static_content.py @@ -13,7 +13,7 @@ import os import sys import textwrap from collections import defaultdict -from pkg_resources import resource_string +from pkg_resources import resource_filename import django from docopt import docopt @@ -43,27 +43,27 @@ class VideoBlock(HTMLSnippet): # lint-amnesty, pylint: disable=abstract-method preview_view_js = { 'js': [ - resource_string(__name__, 'js/src/video/10_main.js'), + resource_filename(__name__, 'js/src/video/10_main.js'), ], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js') + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js') } preview_view_css = { 'scss': [ - resource_string(__name__, 'css/video/display.scss'), - resource_string(__name__, 'css/video/accessible_menu.scss'), + resource_filename(__name__, 'css/video/display.scss'), + resource_filename(__name__, 'css/video/accessible_menu.scss'), ], } studio_view_js = { 'js': [ - resource_string(__name__, 'js/src/tabs/tabs-aggregator.js'), + resource_filename(__name__, 'js/src/tabs/tabs-aggregator.js'), ], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), } studio_view_css = { 'scss': [ - resource_string(__name__, 'css/tabs/tabs.scss'), + resource_filename(__name__, 'css/tabs/tabs.scss'), ] } @@ -132,7 +132,9 @@ def _write_styles(selector, output_root, classes, css_attribute, suffix): for class_ in classes: class_css = getattr(class_, css_attribute)() for filetype in ('sass', 'scss', 'css'): - for idx, fragment in enumerate(class_css.get(filetype, [])): + for idx, fragment_path in enumerate(class_css.get(filetype, [])): + with open(fragment_path, 'rb') as fragment_file: + fragment = fragment_file.read() css_fragments[idx, filetype, fragment].add(class_.__name__) css_imports = defaultdict(set) for (idx, filetype, fragment), classes in sorted(css_fragments.items()): # lint-amnesty, pylint: disable=redefined-argument-from-local @@ -177,10 +179,14 @@ def _write_js(output_root, classes, js_attribute): fragment_owners = defaultdict(list) for class_ in classes: module_js = getattr(class_, js_attribute)() + with open(module_js.get('xmodule_js'), 'rb') as xmodule_js_file: + xmodule_js_fragment = xmodule_js_file.read() # It will enforce 000 prefix for xmodule.js. - fragment_owners[(0, 'js', module_js.get('xmodule_js'))].append(getattr(class_, js_attribute + '_bundle_name')()) + fragment_owners[(0, 'js', xmodule_js_fragment)].append(getattr(class_, js_attribute + '_bundle_name')()) for filetype in ('coffee', 'js'): - for idx, fragment in enumerate(module_js.get(filetype, [])): + for idx, fragment_path in enumerate(module_js.get(filetype, [])): + with open(fragment_path, 'rb') as fragment_file: + fragment = fragment_file.read() fragment_owners[(idx + 1, filetype, fragment)].append(getattr(class_, js_attribute + '_bundle_name')()) for (idx, filetype, fragment), owners in sorted(fragment_owners.items()): diff --git a/xmodule/template_block.py b/xmodule/template_block.py index 70bafca775..71b2c21f14 100644 --- a/xmodule/template_block.py +++ b/xmodule/template_block.py @@ -6,7 +6,7 @@ from string import Template from xblock.core import XBlock from lxml import etree -from pkg_resources import resource_string +from pkg_resources import resource_filename from web_fragments.fragment import Fragment from xmodule.editing_block import EditingMixin from xmodule.raw_block import RawMixin @@ -67,17 +67,17 @@ class CustomTagBlock(CustomTagTemplateBlock): # pylint: disable=abstract-method preview_view_js = { 'js': [], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), } preview_view_css = { 'scss': [], } studio_view_js = { - 'js': [resource_string(__name__, 'js/src/raw/edit/xml.js')], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + 'js': [resource_filename(__name__, 'js/src/raw/edit/xml.js')], + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), } studio_view_css = { - 'scss': [resource_string(__name__, 'css/codemirror/codemirror.scss')], + 'scss': [resource_filename(__name__, 'css/codemirror/codemirror.scss')], } def studio_view(self, _context): diff --git a/xmodule/word_cloud_block.py b/xmodule/word_cloud_block.py index 2e7fadd0a2..d7d35dedc5 100644 --- a/xmodule/word_cloud_block.py +++ b/xmodule/word_cloud_block.py @@ -10,7 +10,7 @@ If student have answered - words he entered and cloud. import json import logging -from pkg_resources import resource_string +from pkg_resources import resource_filename from web_fragments.fragment import Fragment from xblock.core import XBlock @@ -114,21 +114,21 @@ class WordCloudBlock( # pylint: disable=abstract-method preview_view_js = { 'js': [ - resource_string(__name__, 'assets/word_cloud/src/js/word_cloud.js'), + resource_filename(__name__, 'assets/word_cloud/src/js/word_cloud.js'), ], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), } preview_view_css = { 'scss': [ - resource_string(__name__, 'css/word_cloud/display.scss'), + resource_filename(__name__, 'css/word_cloud/display.scss'), ], } studio_view_js = { 'js': [ - resource_string(__name__, 'js/src/raw/edit/metadata-only.js'), + resource_filename(__name__, 'js/src/raw/edit/metadata-only.js'), ], - 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), } studio_view_css = { 'scss': [], From 0e5c3a5b1394b71eb1bd587004317e8995d4893e Mon Sep 17 00:00:00 2001 From: Alexander J Sheehan Date: Thu, 25 May 2023 16:50:00 +0000 Subject: [PATCH 4/5] chore: bumping enterprise package version to 3.65.1 --- lms/envs/common.py | 2 ++ requirements/constraints.txt | 2 +- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/testing.txt | 2 +- 5 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lms/envs/common.py b/lms/envs/common.py index 9cb75c7c4f..4d1b2a876f 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -54,6 +54,7 @@ from enterprise.constants import ( ENTERPRISE_CATALOG_ADMIN_ROLE, ENTERPRISE_DASHBOARD_ADMIN_ROLE, ENTERPRISE_ENROLLMENT_API_ADMIN_ROLE, + ENTERPRISE_FULFILLMENT_OPERATOR_ROLE, ENTERPRISE_REPORTING_CONFIG_ADMIN_ROLE, ENTERPRISE_OPERATOR_ROLE ) @@ -4633,6 +4634,7 @@ SYSTEM_TO_FEATURE_ROLE_MAPPING = { ENTERPRISE_CATALOG_ADMIN_ROLE, ENTERPRISE_ENROLLMENT_API_ADMIN_ROLE, ENTERPRISE_REPORTING_CONFIG_ADMIN_ROLE, + ENTERPRISE_FULFILLMENT_OPERATOR_ROLE, ], } diff --git a/requirements/constraints.txt b/requirements/constraints.txt index ed4b01da01..ae580a05e9 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -25,7 +25,7 @@ django-storages<1.9 # The team that owns this package will manually bump this package rather than having it pulled in automatically. # This is to allow them to better control its deployment and to do it in a process that works better # for them. -edx-enterprise==3.63.0 +edx-enterprise==3.65.1 # oauthlib>3.0.1 causes test failures ( also remove the django-oauth-toolkit constraint when this is fixed ) oauthlib==3.0.1 diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index e9ad77fd97..f3b584ce13 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -483,7 +483,7 @@ edx-drf-extensions==8.8.0 # edx-when # edxval # learner-pathway-progress -edx-enterprise==3.63.0 +edx-enterprise==3.65.1 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index f42d50d787..d9fc8ede2b 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -607,7 +607,7 @@ edx-drf-extensions==8.8.0 # edx-when # edxval # learner-pathway-progress -edx-enterprise==3.63.0 +edx-enterprise==3.65.1 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 2f0a79d3df..8e77798df7 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -585,7 +585,7 @@ edx-drf-extensions==8.8.0 # edx-when # edxval # learner-pathway-progress -edx-enterprise==3.63.0 +edx-enterprise==3.65.1 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt From 73c2eacf7abf73f8428b0c2eea784beae615f6e0 Mon Sep 17 00:00:00 2001 From: irfanuddinahmad Date: Thu, 25 May 2023 18:37:53 +0000 Subject: [PATCH 5/5] feat: Upgrade Python dependency openedx-filters Adds a filter to modify course_home_url Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/testing.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index f3b584ce13..c5e6ca2605 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -779,7 +779,7 @@ openedx-events==8.0.0 # -r requirements/edx/base.in # edx-event-bus-kafka # edx-event-bus-redis -openedx-filters==1.2.0 +openedx-filters==1.3.0 # via # -r requirements/edx/base.in # lti-consumer-xblock diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index d9fc8ede2b..5283bd1765 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1037,7 +1037,7 @@ openedx-events==8.0.0 # -r requirements/edx/testing.txt # edx-event-bus-kafka # edx-event-bus-redis -openedx-filters==1.2.0 +openedx-filters==1.3.0 # via # -r requirements/edx/testing.txt # lti-consumer-xblock diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 8e77798df7..464463ac94 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -984,7 +984,7 @@ openedx-events==8.0.0 # -r requirements/edx/base.txt # edx-event-bus-kafka # edx-event-bus-redis -openedx-filters==1.2.0 +openedx-filters==1.3.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock