${_("This course was created as a re-run. Some manual
${_("This course run is using an upgraded version of edx discussion forum. In order to display the discussions sidebar, discussions xBlocks will no longer be visible to learners.")}
- % if use_v2_cert_display_settings:
-
- % else:
-
- % endif
+
${_("Certificates are awarded at the end of a course run")}
- % if use_v2_cert_display_settings:
-
-
-
-
-
- ${_("Read more about this setting")}
-
+
+
+
+
+
+ ${_("Read more about this setting")}
+
+
+
+
${_("In all configurations of this setting, certificates are generated for learners as soon as they achieve the passing threshold in the course (which can occur before a final assignment based on course design)")}
+
+
${_("Immediately upon passing")}
+
${_("Learners can access their certificate as soon as they achieve a passing grade above the course grade threshold. Note: learners can achieve a passing grade before encountering all assignments in some course configurations.")}
-
-
${_("In all configurations of this setting, certificates are generated for learners as soon as they achieve the passing threshold in the course (which can occur before a final assignment based on course design)")}
-
-
${_("Immediately upon passing")}
-
${_("Learners can access their certificate as soon as they achieve a passing grade above the course grade threshold. Note: learners can achieve a passing grade before encountering all assignments in some course configurations.")}
-
-
-
${_("On course end date")}
-
${_("Learners with passing grades can access their certificate once the end date of the course has elapsed.")}
-
-
-
${_("A date after the course end date")}
-
${_("Learners with passing grades can access their certificate after the date that you set has elapsed.")}
-
+
+
${_("On course end date")}
+
${_("Learners with passing grades can access their certificate once the end date of the course has elapsed.")}
+
+
+
${_("A date after the course end date")}
+
${_("Learners with passing grades can access their certificate after the date that you set has elapsed.")}
- % endif
+
- % if use_v2_cert_display_settings:
-
- % else:
-
- % endif
+
diff --git a/common/djangoapps/course_modes/tests/test_views.py b/common/djangoapps/course_modes/tests/test_views.py
index f49d1bc23b7b..d7b069865317 100644
--- a/common/djangoapps/course_modes/tests/test_views.py
+++ b/common/djangoapps/course_modes/tests/test_views.py
@@ -149,50 +149,6 @@ def test_no_id_redirect_otto(self):
self.assertRedirects(response, '/test_basket/add/?sku=TEST', fetch_redirect_response=False)
ecomm_test_utils.update_commerce_config(enabled=False)
- def test_verified_mode_response_contains_course_run_key(self):
- # Create only the verified mode and enroll the user
- CourseModeFactory.create(
- mode_slug='verified',
- course_id=self.course_that_started.id,
- min_price=149,
- sku="dummy"
- )
- CourseEnrollmentFactory(
- is_active=True,
- course_id=self.course_that_started.id,
- user=self.user
- )
-
- # Value Prop TODO (REV-2378): remove waffle flag from tests once the new Track Selection template is rolled out.
- with override_waffle_flag(VALUE_PROP_TRACK_SELECTION_FLAG, active=True):
- with patch(GATING_METHOD_NAME, return_value=True):
- with patch(CDL_METHOD_NAME, return_value=True):
- with patch("common.djangoapps.course_modes.views.EcommerceService.is_enabled", return_value=True):
- url = reverse('course_modes_choose', args=[str(self.course_that_started.id)])
- response = self.client.get(url)
- self.assertContains(response, "&course_run_key=")
- self.assertContains(response, self.course_that_started.id)
-
- def test_response_without_verified_sku_does_not_contain_course_run_key(self):
- CourseModeFactory.create(
- mode_slug='verified',
- course_id=self.course_that_started.id,
- )
- CourseEnrollmentFactory(
- is_active=True,
- course_id=self.course_that_started.id,
- user=self.user
- )
-
- # Value Prop TODO (REV-2378): remove waffle flag from tests once the new Track Selection template is rolled out.
- with override_waffle_flag(VALUE_PROP_TRACK_SELECTION_FLAG, active=True):
- with patch(GATING_METHOD_NAME, return_value=True):
- with patch(CDL_METHOD_NAME, return_value=True):
- with patch("common.djangoapps.course_modes.views.EcommerceService.is_enabled", return_value=True):
- url = reverse('course_modes_choose', args=[str(self.course_that_started.id)])
- response = self.client.get(url)
- self.assertNotContains(response, "&course_run_key=")
-
@httpretty.activate
@ddt.data(
'',
diff --git a/common/djangoapps/course_modes/views.py b/common/djangoapps/course_modes/views.py
index 821c59dc0524..759073a13583 100644
--- a/common/djangoapps/course_modes/views.py
+++ b/common/djangoapps/course_modes/views.py
@@ -241,7 +241,7 @@ def get(self, request, course_id, error=None): # lint-amnesty, pylint: disable=
if verified_mode.sku:
context["use_ecommerce_payment_flow"] = ecommerce_service.is_enabled(request.user)
- context["ecommerce_payment_page"] = ecommerce_service.get_add_to_basket_url()
+ context["ecommerce_payment_page"] = ecommerce_service.payment_page_url()
context["sku"] = verified_mode.sku
context["bulk_sku"] = verified_mode.bulk_sku
diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py
index fb0a7236c0b5..b5d46949b506 100644
--- a/common/djangoapps/student/helpers.py
+++ b/common/djangoapps/student/helpers.py
@@ -646,21 +646,14 @@ def _is_certificate_earned_but_not_available(course_overview, status):
(bool): True if the user earned the certificate but it's hidden due to display behavior, else False
"""
- if settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS"):
- return (
- not certificates_viewable_for_course(course_overview)
- and CertificateStatuses.is_passing_status(status)
- and course_overview.certificates_display_behavior in (
- CertificatesDisplayBehaviors.END_WITH_DATE,
- CertificatesDisplayBehaviors.END
- )
- )
- else:
- return (
- not certificates_viewable_for_course(course_overview) and
- CertificateStatuses.is_passing_status(status) and
- course_overview.certificate_available_date
+ return (
+ not certificates_viewable_for_course(course_overview)
+ and CertificateStatuses.is_passing_status(status)
+ and course_overview.certificates_display_behavior in (
+ CertificatesDisplayBehaviors.END_WITH_DATE,
+ CertificatesDisplayBehaviors.END
)
+ )
def process_survey_link(survey_link, user):
diff --git a/common/djangoapps/third_party_auth/README.rst b/common/djangoapps/third_party_auth/README.rst
new file mode 100644
index 000000000000..d2e1089eca5d
--- /dev/null
+++ b/common/djangoapps/third_party_auth/README.rst
@@ -0,0 +1,11 @@
+Third Party Auth
+----------------
+
+This djangoapp provides the views and workflows for authenticating into edx-platform with third-party applications, including both OAuth and SAML workflows.
+
+We make use of the `social-auth-app-django`_ as our backend library for this djangoapp.
+
+To enable this feature, check out the `third party authentication documentation`.
+
+.. _social-auth-app-django: https://github.com/python-social-auth/social-app-django
+.. _third party authentication documentation: https://edx.readthedocs.io/projects/edx-installing-configuring-and-running/en/latest/configuration/tpa/index.html
diff --git a/common/djangoapps/third_party_auth/admin.py b/common/djangoapps/third_party_auth/admin.py
index 284c50fcf884..d44da6248ee2 100644
--- a/common/djangoapps/third_party_auth/admin.py
+++ b/common/djangoapps/third_party_auth/admin.py
@@ -53,13 +53,15 @@ class SAMLProviderConfigForm(forms.ModelForm):
class SAMLProviderConfigAdmin(KeyedConfigurationModelAdmin):
""" Django Admin class for SAMLProviderConfig """
form = SAMLProviderConfigForm
+ search_fields = ['display_name']
def get_queryset(self, request):
"""
- Filter the queryset to exclude the archived records.
+ Filter the queryset to exclude the archived records unless it's the /change/ view.
"""
- queryset = super().get_queryset(request).exclude(archived=True)
- return queryset
+ if request.path.endswith('/change/'):
+ return self.model.objects.all()
+ return super().get_queryset(request).exclude(archived=True)
def archive_provider_configuration(self, request, queryset):
"""
@@ -99,7 +101,15 @@ def name_with_update_link(self, instance):
Record name with link for the change view.
"""
if not instance.is_active:
- return instance.name
+ update_url = reverse(
+ f'admin:{self.model._meta.app_label}_{self.model._meta.model_name}_change',
+ args=[instance.pk]
+ )
+ return format_html(
+ '{}',
+ update_url,
+ f'{instance.name}'
+ )
update_url = reverse(f'admin:{self.model._meta.app_label}_{self.model._meta.model_name}_add')
update_url += f'?source={instance.pk}'
@@ -167,11 +177,11 @@ def upload_csv(self, request, slug):
# Always redirect back to the SAMLProviderConfig listing page
return HttpResponseRedirect(reverse('admin:third_party_auth_samlproviderconfig_changelist'))
- def change_view(self, request, object_slug, form_url='', extra_context=None):
+ def change_view(self, request, object_id, form_url='', extra_context=None):
""" Extend the change view to include CSV upload. """
extra_context = extra_context or {}
extra_context['show_csv_upload'] = True
- return super().change_view(request, object_slug, form_url, extra_context)
+ return super().change_view(request, object_id, form_url, extra_context)
def csv_uuid_update_button(self, obj):
""" Add CSV upload button to the form. """
diff --git a/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py b/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py
index 4bfc710fe901..c19b0b8d96aa 100644
--- a/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py
+++ b/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py
@@ -583,7 +583,7 @@ def test_verification_signal(self):
"""
Verification signal is sent upon approval.
"""
- with mock.patch('openedx_events.learning.signals.IDV_ATTEMPT_APPROVED.send_event') as mock_signal:
+ with mock.patch('openedx.core.djangoapps.signals.signals.LEARNER_SSO_VERIFIED.send_robust') as mock_signal:
# Begin the pipeline.
pipeline.set_id_verification_status(
auth_entry=pipeline.AUTH_ENTRY_LOGIN,
diff --git a/common/static/data/geoip/GeoLite2-Country.mmdb b/common/static/data/geoip/GeoLite2-Country.mmdb
index 2e9d48109648..479048791da5 100644
Binary files a/common/static/data/geoip/GeoLite2-Country.mmdb and b/common/static/data/geoip/GeoLite2-Country.mmdb differ
diff --git a/cms/templates/content_libraries/xblock_iframe.html b/common/templates/xblock_v2/xblock_iframe.html
similarity index 99%
rename from cms/templates/content_libraries/xblock_iframe.html
rename to common/templates/xblock_v2/xblock_iframe.html
index b6e455f78515..8b733373bd82 100644
--- a/cms/templates/content_libraries/xblock_iframe.html
+++ b/common/templates/xblock_v2/xblock_iframe.html
@@ -156,7 +156,7 @@
-
+
{{ fragment.body_html | safe }}
diff --git a/docs/docs_settings.py b/docs/docs_settings.py
index 5bc9b1594697..f12848876e8a 100644
--- a/docs/docs_settings.py
+++ b/docs/docs_settings.py
@@ -14,7 +14,6 @@
ADVANCED_PROBLEM_TYPES,
COURSE_IMPORT_EXPORT_STORAGE,
GIT_EXPORT_DEFAULT_IDENT,
- LIBRARY_AUTHORING_MICROFRONTEND_URL,
SCRAPE_YOUTUBE_THUMBNAILS_JOB_QUEUE,
VIDEO_TRANSCRIPT_MIGRATIONS_JOB_QUEUE,
UPDATE_SEARCH_INDEX_JOB_QUEUE,
diff --git a/lms/djangoapps/branding/tests/test_page.py b/lms/djangoapps/branding/tests/test_page.py
index 9904f79f644f..46f10bd6923a 100644
--- a/lms/djangoapps/branding/tests/test_page.py
+++ b/lms/djangoapps/branding/tests/test_page.py
@@ -22,6 +22,7 @@
from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order
+from xmodule.course_block import CATALOG_VISIBILITY_ABOUT, CATALOG_VISIBILITY_NONE
FEATURES_WITH_STARTDATE = settings.FEATURES.copy()
FEATURES_WITH_STARTDATE['DISABLE_START_DATES'] = False
@@ -201,6 +202,20 @@ def setUp(self):
display_name='Tech Beta Course',
emit_signals=True,
)
+ self.course_with_none_visibility = CourseFactory.create(
+ org='MITx',
+ number='1003',
+ catalog_visibility=CATALOG_VISIBILITY_NONE,
+ display_name='Course with "none" catalog visibility',
+ emit_signals=True,
+ )
+ self.course_with_about_visibility = CourseFactory.create(
+ org='MITx',
+ number='1003',
+ catalog_visibility=CATALOG_VISIBILITY_ABOUT,
+ display_name='Course with "about" catalog visibility',
+ emit_signals=True,
+ )
self.factory = RequestFactory()
@patch('common.djangoapps.student.views.management.render_to_response', RENDER_MOCK)
@@ -300,6 +315,15 @@ def test_course_cards_sorted_by_start_date_disabled(self):
assert context['courses'][1].id == self.starting_earlier.id
assert context['courses'][2].id == self.course_with_default_start_date.id
+ @patch('lms.djangoapps.courseware.views.views.render_to_response', RENDER_MOCK)
+ def test_invisible_courses_are_not_displayed(self):
+ response = self.client.get(reverse('courses'))
+ ((_template, context), _) = RENDER_MOCK.call_args # pylint: disable=unpacking-non-sequence
+
+ rendered_ids = [course.id for course in context["courses"]]
+ assert self.course_with_none_visibility.id not in rendered_ids
+ assert self.course_with_about_visibility.id not in rendered_ids
+
class IndexPageProgramsTests(SiteMixin, ModuleStoreTestCase):
"""
diff --git a/lms/djangoapps/bulk_email/message_types.py b/lms/djangoapps/bulk_email/message_types.py
index 033f5423e1f7..53c7e92029a5 100644
--- a/lms/djangoapps/bulk_email/message_types.py
+++ b/lms/djangoapps/bulk_email/message_types.py
@@ -12,3 +12,4 @@ class BulkEmail(BaseMessageType):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.options['from_address'] = kwargs['context']['from_address']
+ self.options['transactional'] = True
diff --git a/lms/djangoapps/certificates/api.py b/lms/djangoapps/certificates/api.py
index 4439eeb5f220..bd7db8662e70 100644
--- a/lms/djangoapps/certificates/api.py
+++ b/lms/djangoapps/certificates/api.py
@@ -10,7 +10,6 @@
import logging
from datetime import datetime
-from django.conf import settings
from django.contrib.auth import get_user_model
from django.core.exceptions import ObjectDoesNotExist
from django.db.models import Q
@@ -286,12 +285,9 @@ def certificate_downloadable_status(student, course_key):
course_overview = get_course_overview_or_none(course_key)
- if settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS"):
- display_behavior_is_valid = (
- course_overview.certificates_display_behavior == CertificatesDisplayBehaviors.END_WITH_DATE
- )
- else:
- display_behavior_is_valid = True
+ display_behavior_is_valid = (
+ course_overview.certificates_display_behavior == CertificatesDisplayBehaviors.END_WITH_DATE
+ )
if (
not certificates_viewable_for_course(course_overview)
@@ -837,10 +833,7 @@ def can_show_certificate_message(course, student, course_grade, certificates_ena
def _course_uses_available_date(course):
"""Returns if the course has an certificate_available_date set and that it should be used"""
- if settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS"):
- display_behavior_is_valid = course.certificates_display_behavior == CertificatesDisplayBehaviors.END_WITH_DATE
- else:
- display_behavior_is_valid = True
+ display_behavior_is_valid = course.certificates_display_behavior == CertificatesDisplayBehaviors.END_WITH_DATE
return (
can_show_certificate_available_date_field(course)
diff --git a/lms/djangoapps/certificates/docs/diagrams/certificate_generation.dsl b/lms/djangoapps/certificates/docs/diagrams/certificate_generation.dsl
index d7ca8fd9a400..e5b66bf3b2ab 100644
--- a/lms/djangoapps/certificates/docs/diagrams/certificate_generation.dsl
+++ b/lms/djangoapps/certificates/docs/diagrams/certificate_generation.dsl
@@ -32,6 +32,8 @@ workspace {
grades_app -> signal_handlers "Emits COURSE_GRADE_NOW_PASSED signal"
verify_student_app -> signal_handlers "Emits IDV_ATTEMPT_APPROVED signal"
+ verify_student_app -> signal_handlers "Emits LEARNER_SSO_VERIFIED signal"
+ verify_student_app -> signal_handlers "Emits PHOTO_VERIFICATION_APPROVED signal"
student_app -> signal_handlers "Emits ENROLLMENT_TRACK_UPDATED signal"
allowlist -> signal_handlers "Emits APPEND_CERTIFICATE_ALLOWLIST signal"
signal_handlers -> generation_handler "Invokes generate_allowlist_certificate()"
diff --git a/lms/djangoapps/certificates/signals.py b/lms/djangoapps/certificates/signals.py
index 53055bf9c86e..39042ff34164 100644
--- a/lms/djangoapps/certificates/signals.py
+++ b/lms/djangoapps/certificates/signals.py
@@ -32,6 +32,8 @@
from openedx.core.djangoapps.signals.signals import (
COURSE_GRADE_NOW_FAILED,
COURSE_GRADE_NOW_PASSED,
+ LEARNER_SSO_VERIFIED,
+ PHOTO_VERIFICATION_APPROVED,
)
from openedx_events.learning.signals import EXAM_ATTEMPT_REJECTED, IDV_ATTEMPT_APPROVED
@@ -117,17 +119,13 @@ def _listen_for_failing_grade(sender, user, course_id, grade, **kwargs): # pyli
log.info(f'Certificate marked not passing for {user.id} : {course_id} via failing grade')
-@receiver(IDV_ATTEMPT_APPROVED, dispatch_uid="learner_track_changed")
-def _listen_for_id_verification_status_changed(sender, signal, **kwargs): # pylint: disable=unused-argument
+def _handle_id_verification_approved(user):
"""
- Listen for a signal indicating that the user's id verification status has changed.
+ Generate a certificate for the user if they are now verified
"""
if not auto_certificate_generation_enabled():
return
- event_data = kwargs.get('idv_attempt')
- user = User.objects.get(id=event_data.user.id)
-
user_enrollments = CourseEnrollment.enrollments_for_user(user=user)
expected_verification_status = IDVerificationService.user_status(user)
expected_verification_status = expected_verification_status['status']
@@ -145,6 +143,25 @@ def _listen_for_id_verification_status_changed(sender, signal, **kwargs): # pyl
)
+@receiver(LEARNER_SSO_VERIFIED, dispatch_uid="sso_learner_verified")
+@receiver(PHOTO_VERIFICATION_APPROVED, dispatch_uid="photo_verification_approved")
+def _listen_for_sso_verification_approved(sender, user, **kwargs): # pylint: disable=unused-argument
+ """
+ Listen for a signal on SSOVerification indicating that the user has been verified.
+ """
+ _handle_id_verification_approved(user)
+
+
+@receiver(IDV_ATTEMPT_APPROVED, dispatch_uid="openedx_idv_attempt_approved")
+def _listen_for_id_verification_approved_event(sender, signal, **kwargs): # pylint: disable=unused-argument
+ """
+ Listen for an openedx event indicating that the user's id verification status has changed.
+ """
+ event_data = kwargs.get('idv_attempt')
+ user = User.objects.get(id=event_data.user.id)
+ _handle_id_verification_approved(user)
+
+
@receiver(ENROLLMENT_TRACK_UPDATED)
def _listen_for_enrollment_mode_change(sender, user, course_key, mode, **kwargs): # pylint: disable=unused-argument
"""
diff --git a/lms/djangoapps/certificates/tests/test_api.py b/lms/djangoapps/certificates/tests/test_api.py
index 21d04a7e3da6..cb11b9e00bd1 100644
--- a/lms/djangoapps/certificates/tests/test_api.py
+++ b/lms/djangoapps/certificates/tests/test_api.py
@@ -209,29 +209,6 @@ def test_with_downloadable_web_cert(self):
"uuid": cert_status["uuid"],
}
- @ddt.data(
- (False, timedelta(days=2), False, True),
- (False, -timedelta(days=2), True, None),
- (True, timedelta(days=2), True, None),
- )
- @ddt.unpack
- @patch.dict(settings.FEATURES, {"CERTIFICATES_HTML_VIEW": True})
- @patch.dict(settings.FEATURES, {"ENABLE_V2_CERT_DISPLAY_SETTINGS": False})
- def test_cert_api_return_v1(self, self_paced, cert_avail_delta, cert_downloadable_status, earned_but_not_available):
- """
- Test 'downloadable status'
- """
- cert_avail_date = datetime.now(pytz.UTC) + cert_avail_delta
- self.course.self_paced = self_paced
- self.course.certificate_available_date = cert_avail_date
- self.course.save()
-
- self._setup_course_certificate()
-
- downloadable_status = certificate_downloadable_status(self.student, self.course.id)
- assert downloadable_status["is_downloadable"] == cert_downloadable_status
- assert downloadable_status.get("earned_but_not_available") == earned_but_not_available
-
@ddt.data(
(True, timedelta(days=2), CertificatesDisplayBehaviors.END_WITH_DATE, True, None),
(False, -timedelta(days=2), CertificatesDisplayBehaviors.EARLY_NO_INFO, True, None),
@@ -243,8 +220,7 @@ def test_cert_api_return_v1(self, self_paced, cert_avail_delta, cert_downloadabl
)
@ddt.unpack
@patch.dict(settings.FEATURES, {"CERTIFICATES_HTML_VIEW": True})
- @patch.dict(settings.FEATURES, {"ENABLE_V2_CERT_DISPLAY_SETTINGS": True})
- def test_cert_api_return_v2(
+ def test_cert_api_return(
self,
self_paced,
cert_avail_delta,
diff --git a/lms/djangoapps/certificates/tests/test_filters.py b/lms/djangoapps/certificates/tests/test_filters.py
index 781b77461f41..a131a1d52582 100644
--- a/lms/djangoapps/certificates/tests/test_filters.py
+++ b/lms/djangoapps/certificates/tests/test_filters.py
@@ -23,7 +23,7 @@
from lms.djangoapps.certificates.models import GeneratedCertificate
from lms.djangoapps.certificates.signals import (
_listen_for_enrollment_mode_change,
- _listen_for_id_verification_status_changed,
+ _handle_id_verification_approved,
listen_for_passing_grade
)
from lms.djangoapps.certificates.tests.factories import CertificateAllowlistFactory
@@ -272,7 +272,7 @@ def test_listen_for_passing_grade(self):
mock.Mock(return_value={"status": "approved"})
)
@mock.patch("lms.djangoapps.certificates.api.auto_certificate_generation_enabled", mock.Mock(return_value=True))
- def test_listen_for_id_verification_status_changed(self):
+ def test_handle_id_verification_approved(self):
"""
Test stop certificate generation process after the verification status changed by raising a filters exception.
@@ -280,7 +280,7 @@ def test_listen_for_id_verification_status_changed(self):
- CertificateCreationRequested is triggered and executes TestStopCertificateGenerationStep.
- The certificate is not generated.
"""
- _listen_for_id_verification_status_changed(None, self.user)
+ _handle_id_verification_approved(self.user)
self.assertFalse(
GeneratedCertificate.objects.filter(
diff --git a/lms/djangoapps/certificates/tests/test_utils.py b/lms/djangoapps/certificates/tests/test_utils.py
index 00bb0bbe3ed0..298e624fdd8e 100644
--- a/lms/djangoapps/certificates/tests/test_utils.py
+++ b/lms/djangoapps/certificates/tests/test_utils.py
@@ -5,7 +5,6 @@
from unittest.mock import patch
import ddt
-from django.conf import settings
from django.test import TestCase
from pytz import utc
@@ -80,40 +79,7 @@ def test_has_html_certificates_enabled_from_course_overview_disabled(self):
(CertificatesDisplayBehaviors.END, False, False, _LAST_MONTH, True, True),
)
@ddt.unpack
- @patch.dict(settings.FEATURES, ENABLE_V2_CERT_DISPLAY_SETTINGS=True)
- def test_should_certificate_be_visible_v2(
- self,
- certificates_display_behavior,
- certificates_show_before_end,
- has_ended,
- certificate_available_date,
- self_paced,
- expected_value
- ):
- """Test whether the certificate should be visible to user given multiple usecases"""
- assert should_certificate_be_visible(
- certificates_display_behavior,
- certificates_show_before_end,
- has_ended,
- certificate_available_date,
- self_paced
- ) == expected_value
-
- @ddt.data(
- ('early_with_info', True, True, _LAST_MONTH, False, True),
- ('early_no_info', False, False, _LAST_MONTH, False, True),
- ('end', True, False, _LAST_MONTH, False, True),
- ('end', False, True, _LAST_MONTH, False, True),
- ('end', False, False, _NEXT_WEEK, False, False),
- ('end', False, False, _LAST_WEEK, False, True),
- ('end', False, False, None, False, False),
- ('early_with_info', False, False, None, False, True),
- ('end', False, False, _NEXT_WEEK, False, False),
- ('end', False, False, _NEXT_WEEK, True, True),
- )
- @ddt.unpack
- @patch.dict(settings.FEATURES, ENABLE_V2_CERT_DISPLAY_SETTINGS=False)
- def test_should_certificate_be_visible_v1(
+ def test_should_certificate_be_visible(
self,
certificates_display_behavior,
certificates_show_before_end,
diff --git a/lms/djangoapps/certificates/utils.py b/lms/djangoapps/certificates/utils.py
index 725c54ac09bc..7ff2a4c97b27 100644
--- a/lms/djangoapps/certificates/utils.py
+++ b/lms/djangoapps/certificates/utils.py
@@ -153,30 +153,19 @@ def should_certificate_be_visible(
certificate_available_date (datetime): the date the certificate is available on for the course.
self_paced (bool): Whether the course is self-paced.
"""
- if settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS"):
- show_early = (
- certificates_display_behavior == CertificatesDisplayBehaviors.EARLY_NO_INFO
- or certificates_show_before_end
- )
- past_available_date = (
- certificates_display_behavior == CertificatesDisplayBehaviors.END_WITH_DATE
- and certificate_available_date
- and certificate_available_date < datetime.now(utc)
- )
- ended_without_available_date = (
- certificates_display_behavior == CertificatesDisplayBehaviors.END
- and has_ended
- )
- else:
- show_early = (
- certificates_display_behavior in ('early_with_info', 'early_no_info')
- or certificates_show_before_end
- )
- past_available_date = (
- certificate_available_date
- and certificate_available_date < datetime.now(utc)
- )
- ended_without_available_date = (certificate_available_date is None) and has_ended
+ show_early = (
+ certificates_display_behavior == CertificatesDisplayBehaviors.EARLY_NO_INFO
+ or certificates_show_before_end
+ )
+ past_available_date = (
+ certificates_display_behavior == CertificatesDisplayBehaviors.END_WITH_DATE
+ and certificate_available_date
+ and certificate_available_date < datetime.now(utc)
+ )
+ ended_without_available_date = (
+ certificates_display_behavior == CertificatesDisplayBehaviors.END
+ and has_ended
+ )
return any((self_paced, show_early, past_available_date, ended_without_available_date))
diff --git a/lms/djangoapps/certificates/views/webview.py b/lms/djangoapps/certificates/views/webview.py
index 40a1f4499305..06e4e8a55337 100644
--- a/lms/djangoapps/certificates/views/webview.py
+++ b/lms/djangoapps/certificates/views/webview.py
@@ -353,28 +353,22 @@ def _get_user_certificate(request, user, course_key, course_overview, preview_mo
if preview_mode:
# certificate is being previewed from studio
if request.user.has_perm(PREVIEW_CERTIFICATES, course_overview):
- if not settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS"):
- if course_overview.certificate_available_date and not course_overview.self_paced:
- modified_date = course_overview.certificate_available_date
- else:
- modified_date = datetime.now().date()
+ if (
+ course_overview.certificates_display_behavior == CertificatesDisplayBehaviors.END_WITH_DATE
+ and course_overview.certificate_available_date
+ and not course_overview.self_paced
+ ):
+ modified_date = course_overview.certificate_available_date
+ elif course_overview.certificates_display_behavior == CertificatesDisplayBehaviors.END:
+ modified_date = course_overview.end
else:
- if (
- course_overview.certificates_display_behavior == CertificatesDisplayBehaviors.END_WITH_DATE
- and course_overview.certificate_available_date
- and not course_overview.self_paced
- ):
- modified_date = course_overview.certificate_available_date
- elif course_overview.certificates_display_behavior == CertificatesDisplayBehaviors.END:
- modified_date = course_overview.end
- else:
- modified_date = datetime.now().date()
- user_certificate = GeneratedCertificate(
- mode=preview_mode,
- verify_uuid=str(uuid4().hex),
- modified_date=modified_date,
- created_date=datetime.now().date(),
- )
+ modified_date = datetime.now().date()
+ user_certificate = GeneratedCertificate(
+ mode=preview_mode,
+ verify_uuid=str(uuid4().hex),
+ modified_date=modified_date,
+ created_date=datetime.now().date(),
+ )
elif certificates_viewable_for_course(course_overview):
# certificate is being viewed by learner or public
try:
diff --git a/lms/djangoapps/commerce/tests/test_utils.py b/lms/djangoapps/commerce/tests/test_utils.py
index 6c793433dff6..d5d0cf1f1c23 100644
--- a/lms/djangoapps/commerce/tests/test_utils.py
+++ b/lms/djangoapps/commerce/tests/test_utils.py
@@ -11,7 +11,6 @@
from django.test import TestCase
from django.test.client import RequestFactory
from django.test.utils import override_settings
-from edx_toggles.toggles.testutils import override_waffle_flag
from opaque_keys.edx.locator import CourseLocator
from waffle.testutils import override_switch
@@ -21,7 +20,6 @@
from common.djangoapps.student.tests.factories import TEST_PASSWORD, UserFactory
from lms.djangoapps.commerce.models import CommerceConfiguration
from lms.djangoapps.commerce.utils import EcommerceService, refund_entitlement, refund_seat
-from lms.djangoapps.commerce.waffle import ENABLE_TRANSITION_TO_COORDINATOR_CHECKOUT
from openedx.core.djangolib.testing.utils import skip_unless_lms
from openedx.core.lib.log_utils import audit_log
from xmodule.modulestore.tests.django_utils import \
@@ -186,27 +184,6 @@ def test_get_checkout_page_url_with_enterprise_catalog_uuid(self, skus, enterpri
assert url == expected_url
- @override_settings(COMMERCE_COORDINATOR_URL_ROOT='http://coordinator_url')
- @override_settings(ECOMMERCE_PUBLIC_URL_ROOT='http://ecommerce_url')
- @ddt.data(
- {'coordinator_flag_active': True},
- {'coordinator_flag_active': False}
- )
- @ddt.unpack
- def test_get_add_to_basket_url(self, coordinator_flag_active):
- with override_waffle_flag(ENABLE_TRANSITION_TO_COORDINATOR_CHECKOUT, active=coordinator_flag_active):
-
- ecommerce_service = EcommerceService()
- result = ecommerce_service.get_add_to_basket_url()
-
- if coordinator_flag_active:
- expected_url = 'http://coordinator_url/lms/payment_page_redirect/'
- else:
- expected_url = 'http://ecommerce_url/test_basket/add/'
-
- self.assertIsNotNone(result)
- self.assertEqual(expected_url, result)
-
@ddt.ddt
@skip_unless_lms
diff --git a/lms/djangoapps/commerce/utils.py b/lms/djangoapps/commerce/utils.py
index 82ed8c483024..9abcabd4a96d 100644
--- a/lms/djangoapps/commerce/utils.py
+++ b/lms/djangoapps/commerce/utils.py
@@ -13,7 +13,6 @@
from django.utils.translation import gettext as _
from common.djangoapps.course_modes.models import CourseMode
-from common.djangoapps.student.models import CourseEnrollmentAttribute
from openedx.core.djangoapps.commerce.utils import (
get_ecommerce_api_base_url,
get_ecommerce_api_client,
@@ -23,10 +22,6 @@
from openedx.core.djangoapps.theming import helpers as theming_helpers
from .models import CommerceConfiguration
-from .waffle import ( # lint-amnesty, pylint: disable=invalid-django-waffle-import
- should_redirect_to_commerce_coordinator_checkout,
- should_redirect_to_commerce_coordinator_refunds,
-)
from edx_django_utils.plugins import pluggable_override
log = logging.getLogger(__name__)
@@ -110,15 +105,6 @@ def payment_page_url(self):
"""
return self.get_absolute_ecommerce_url(self.config.basket_checkout_page)
- def get_add_to_basket_url(self):
- """ Return the URL for the payment page based on the waffle switch.
- Example:
- http://localhost/enabled_service_api_path
- """
- if should_redirect_to_commerce_coordinator_checkout():
- return urljoin(settings.COMMERCE_COORDINATOR_URL_ROOT, settings.COORDINATOR_CHECKOUT_REDIRECT_PATH)
- return self.payment_page_url()
-
@pluggable_override('OVERRIDE_GET_CHECKOUT_PAGE_URL')
def get_checkout_page_url(self, *skus, **kwargs):
""" Construct the URL to the ecommerce checkout page and include products.
@@ -238,6 +224,7 @@ def refund_entitlement(course_entitlement):
return False
+@pluggable_override('OVERRIDE_REFUND_SEAT')
def refund_seat(course_enrollment, change_mode=False):
"""
Attempt to initiate a refund for any orders associated with the seat being unenrolled,
@@ -260,10 +247,6 @@ def refund_seat(course_enrollment, change_mode=False):
course_key_str = str(course_enrollment.course_id)
enrollee = course_enrollment.user
- if should_redirect_to_commerce_coordinator_refunds():
- if _refund_in_commerce_coordinator(course_enrollment, change_mode):
- return
-
service_user = User.objects.get(username=settings.ECOMMERCE_SERVICE_WORKER_USERNAME)
api_client = get_ecommerce_api_client(service_user)
@@ -287,83 +270,14 @@ def refund_seat(course_enrollment, change_mode=False):
user=enrollee,
)
if change_mode:
- _auto_enroll(course_enrollment)
+ auto_enroll(course_enrollment)
else:
log.info('No refund opened for user [%s], course [%s]', enrollee.id, course_key_str)
return refund_ids
-def _refund_in_commerce_coordinator(course_enrollment, change_mode):
- """
- Helper function to perform refund in Commerce Coordinator.
-
- Parameters:
- course_enrollment (CourseEnrollment): the enrollment to refund.
- change_mode (bool): whether the enrollment should be auto-enrolled into
- the default course mode after refund.
-
- Returns:
- bool: True if refund was performed. False if refund is not applicable
- to Commerce Coordinator.
- """
- enrollment_source_system = course_enrollment.get_order_attribute_value("source_system")
- course_key_str = str(course_enrollment.course_id)
-
- # Commerce Coordinator enrollments will have an orders.source_system enrollment attribute.
- # Redirect to Coordinator only if the source_system is safelisted as Coordinator's in settings.
-
- if enrollment_source_system and enrollment_source_system in settings.COMMERCE_COORDINATOR_REFUND_SOURCE_SYSTEMS:
- log.info('Redirecting refund to Commerce Coordinator for user [%s], course [%s]...',
- course_enrollment.user_id, course_key_str)
-
- # Re-use Ecommerce API client factory to build an API client for Commerce Coordinator...
- service_user = get_user_model().objects.get(
- username=settings.COMMERCE_COORDINATOR_SERVICE_WORKER_USERNAME
- )
- api_client = get_ecommerce_api_client(service_user)
- refunds_url = urljoin(
- settings.COMMERCE_COORDINATOR_URL_ROOT,
- settings.COMMERCE_COORDINATOR_REFUND_PATH
- )
-
- # Build request, raising exception if Coordinator returns non-200.
- enrollment_attributes = CourseEnrollmentAttribute.get_enrollment_attributes(course_enrollment)
-
- try:
- api_client.post(
- refunds_url,
- json={
- 'course_id': course_key_str,
- 'username': course_enrollment.username,
- 'enrollment_attributes': enrollment_attributes
- }
- ).raise_for_status()
-
- except Exception as exc: # pylint: disable=broad-except
- # Catch any possible exceptions from the Commerce Coordinator service to ensure we fail gracefully
- log.exception(
- "Unexpected exception while attempting to refund user in Coordinator [%s], "
- "course key [%s] message: [%s]",
- course_enrollment.username,
- course_key_str,
- str(exc)
- )
-
- # Refund was successfully sent to Commerce Coordinator
- log.info('Refund successfully sent to Commerce Coordinator for user [%s], course [%s].',
- course_enrollment.user_id, course_key_str)
- if change_mode:
- _auto_enroll(course_enrollment)
- return True
- else:
- # Refund was not meant to be sent to Commerce Coordinator
- log.info('Continuing refund without Commerce Coordinator redirect for user [%s], course [%s]...',
- course_enrollment.user_id, course_key_str)
- return False
-
-
-def _auto_enroll(course_enrollment):
+def auto_enroll(course_enrollment):
"""
Helper method to update an enrollment to a default course mode.
diff --git a/lms/djangoapps/commerce/waffle.py b/lms/djangoapps/commerce/waffle.py
deleted file mode 100644
index a36586a52d9b..000000000000
--- a/lms/djangoapps/commerce/waffle.py
+++ /dev/null
@@ -1,49 +0,0 @@
-"""
-Configuration for features of Commerce App
-"""
-from edx_toggles.toggles import WaffleFlag
-
-# Namespace for Commerce waffle flags.
-WAFFLE_FLAG_NAMESPACE = "commerce"
-
-# .. toggle_name: commerce.transition_to_coordinator.checkout
-# .. toggle_implementation: WaffleFlag
-# .. toggle_default: False
-# .. toggle_description: Allows to redirect checkout to Commerce Coordinator API
-# .. toggle_use_cases: temporary
-# .. toggle_creation_date: 2023-11-22
-# .. toggle_target_removal_date: TBA
-# .. toggle_tickets: SONIC-99
-# .. toggle_status: supported
-ENABLE_TRANSITION_TO_COORDINATOR_CHECKOUT = WaffleFlag(
- f"{WAFFLE_FLAG_NAMESPACE}.transition_to_coordinator.checkout",
- __name__,
-)
-
-# .. toggle_name: commerce.transition_to_coordinator.refund
-# .. toggle_implementation: WaffleFlag
-# .. toggle_default: False
-# .. toggle_description: Allows to redirect refunds to Commerce Coordinator API
-# .. toggle_use_cases: temporary
-# .. toggle_creation_date: 2024-03-26
-# .. toggle_target_removal_date: TBA
-# .. toggle_tickets: SONIC-382
-# .. toggle_status: supported
-ENABLE_TRANSITION_TO_COORDINATOR_REFUNDS = WaffleFlag(
- f"{WAFFLE_FLAG_NAMESPACE}.transition_to_coordinator.refunds",
- __name__,
-)
-
-
-def should_redirect_to_commerce_coordinator_checkout():
- """
- Redirect learners to Commerce Coordinator checkout.
- """
- return ENABLE_TRANSITION_TO_COORDINATOR_CHECKOUT.is_enabled()
-
-
-def should_redirect_to_commerce_coordinator_refunds():
- """
- Redirect learners to Commerce Coordinator refunds.
- """
- return ENABLE_TRANSITION_TO_COORDINATOR_REFUNDS.is_enabled()
diff --git a/lms/djangoapps/course_blocks/transformers/library_content.py b/lms/djangoapps/course_blocks/transformers/library_content.py
index 616cf68f4b62..10ef8c2138b6 100644
--- a/lms/djangoapps/course_blocks/transformers/library_content.py
+++ b/lms/djangoapps/course_blocks/transformers/library_content.py
@@ -14,7 +14,7 @@
BlockStructureTransformer,
FilteringTransformerMixin
)
-from xmodule.library_content_block import LibraryContentBlock # lint-amnesty, pylint: disable=wrong-import-order
+from xmodule.library_content_block import LegacyLibraryContentBlock # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
from ..utils import get_student_module_as_dict
@@ -47,7 +47,6 @@ def collect(cls, block_structure):
Collects any information that's necessary to execute this
transformer's transform method.
"""
- block_structure.request_xblock_fields('mode')
block_structure.request_xblock_fields('max_count')
block_structure.request_xblock_fields('category')
store = modulestore()
@@ -83,7 +82,6 @@ def transform_block_filters(self, usage_info, block_structure):
if library_children:
all_library_children.update(library_children)
selected = []
- mode = block_structure.get_xblock_field(block_key, 'mode')
max_count = block_structure.get_xblock_field(block_key, 'max_count')
if max_count < 0:
max_count = len(library_children)
@@ -100,7 +98,7 @@ def transform_block_filters(self, usage_info, block_structure):
# Update selected
previous_count = len(selected)
- block_keys = LibraryContentBlock.make_selection(selected, library_children, max_count, mode)
+ block_keys = LegacyLibraryContentBlock.make_selection(selected, library_children, max_count)
selected = block_keys['selected']
# Save back any changes
@@ -176,7 +174,7 @@ def publish_event(event_name, result, **kwargs):
with tracker.get_tracker().context(full_event_name, context):
tracker.emit(full_event_name, event_data)
- LibraryContentBlock.publish_selected_children_events(
+ LegacyLibraryContentBlock.publish_selected_children_events(
block_keys,
format_block_keys,
publish_event,
diff --git a/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py b/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py
index 0f8227e3201a..abd1dc53755b 100644
--- a/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py
+++ b/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py
@@ -23,7 +23,7 @@
from openedx.core.djangoapps.user_api.preferences.api import get_user_preference
from openedx.core.lib.celery.task_utils import emulate_http_request
from openedx.features.course_duration_limits.access import get_user_course_expiration_date
-from openedx.features.course_experience import ENABLE_COURSE_GOALS
+from openedx.features.course_experience import ENABLE_COURSE_GOALS, ENABLE_SES_FOR_GOALREMINDER
from openedx.features.course_experience.url_helpers import get_learning_mfe_home_url
log = logging.getLogger(__name__)
@@ -86,13 +86,24 @@ def send_ace_message(goal):
'programs_url': getattr(settings, 'ACE_EMAIL_PROGRAMS_URL', None),
})
+ options = {'transactional': True}
+
+ is_ses_enabled = ENABLE_SES_FOR_GOALREMINDER.is_enabled(goal.course_key)
+
+ if is_ses_enabled:
+ options = {
+ 'transactional': True,
+ 'from_address': settings.LMS_COMM_DEFAULT_FROM_EMAIL,
+ 'override_default_channel': 'django_email',
+ }
+
msg = Message(
name="goalreminder",
app_label="course_goals",
recipient=Recipient(user.id, user.email),
language=language,
context=message_context,
- options={'transactional': True},
+ options=options,
)
with emulate_http_request(site, user):
diff --git a/lms/djangoapps/course_goals/management/commands/tests/test_goal_reminder_email.py b/lms/djangoapps/course_goals/management/commands/tests/test_goal_reminder_email.py
index ad60420e0d30..5b98b202d41f 100644
--- a/lms/djangoapps/course_goals/management/commands/tests/test_goal_reminder_email.py
+++ b/lms/djangoapps/course_goals/management/commands/tests/test_goal_reminder_email.py
@@ -5,6 +5,7 @@
from unittest import mock # lint-amnesty, pylint: disable=wrong-import-order
import ddt
+from django.conf import settings
from django.core.management import call_command
from django.test import TestCase
from edx_toggles.toggles.testutils import override_waffle_flag
@@ -20,7 +21,7 @@
from lms.djangoapps.certificates.data import CertificateStatuses
from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory
from openedx.core.djangolib.testing.utils import skip_unless_lms
-from openedx.features.course_experience import ENABLE_COURSE_GOALS
+from openedx.features.course_experience import ENABLE_COURSE_GOALS, ENABLE_SES_FOR_GOALREMINDER
# Some constants just for clarity of tests (assuming week starts on a Monday, as March 2021 used below does)
MONDAY = 0
@@ -180,3 +181,33 @@ def test_no_days_per_week(self):
def test_old_course(self, end):
self.make_valid_goal(overview__end=end)
self.call_command(expect_sent=False)
+
+ @mock.patch('lms.djangoapps.course_goals.management.commands.goal_reminder_email.ace.send')
+ def test_params_with_ses(self, mock_ace):
+ """Test that the parameters of the msg passed to ace.send() are set correctly when SES is enabled"""
+ with override_waffle_flag(ENABLE_SES_FOR_GOALREMINDER, active=None):
+ goal = self.make_valid_goal()
+ flag = get_waffle_flag_model().get(ENABLE_SES_FOR_GOALREMINDER.name)
+ flag.users.add(goal.user)
+
+ with freeze_time('2021-03-02 10:00:00'):
+ call_command('goal_reminder_email')
+
+ assert mock_ace.call_count == 1
+ msg = mock_ace.call_args[0][0]
+ assert msg.options['override_default_channel'] == 'django_email'
+ assert msg.options['from_address'] == settings.LMS_COMM_DEFAULT_FROM_EMAIL
+
+ @mock.patch('lms.djangoapps.course_goals.management.commands.goal_reminder_email.ace.send')
+ def test_params_without_ses(self, mock_ace):
+ """Test that the parameters of the msg passed to ace.send() are set correctly when SES is not enabled"""
+ self.make_valid_goal()
+
+ with freeze_time('2021-03-02 10:00:00'):
+ call_command('goal_reminder_email')
+
+ assert mock_ace.call_count == 1
+ msg = mock_ace.call_args[0][0]
+ assert msg.options['transactional'] is True
+ assert 'override_default_channel' not in msg.options
+ assert 'from_address' not in msg.options
diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py
index 1bae90322487..de92692ce4fc 100644
--- a/lms/djangoapps/courseware/block_render.py
+++ b/lms/djangoapps/courseware/block_render.py
@@ -45,7 +45,7 @@
from openedx.core.lib.xblock_services.call_to_action import CallToActionService
from xmodule.contentstore.django import contentstore
from xmodule.exceptions import NotFoundError, ProcessingError
-from xmodule.library_tools import LibraryToolsService
+from xmodule.library_tools import LegacyLibraryToolsService
from xmodule.modulestore.django import XBlockI18nService, modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.partitions.partitions_service import PartitionService
@@ -626,7 +626,7 @@ def inner_get_block(block: XBlock) -> XBlock | None:
),
'completion': CompletionService(user=user, context_key=course_id) if user and user.is_authenticated else None,
'i18n': XBlockI18nService,
- 'library_tools': LibraryToolsService(store, user_id=user.id if user else None),
+ 'library_tools': LegacyLibraryToolsService(store, user_id=user.id if user else None),
'partitions': PartitionService(course_id=course_id, cache=DEFAULT_REQUEST_CACHE.data),
'settings': SettingsService(),
'user_tags': UserTagsService(user=user, course_id=course_id),
diff --git a/lms/djangoapps/courseware/toggles.py b/lms/djangoapps/courseware/toggles.py
index 43fb40436a5e..e6070a2e3bd1 100644
--- a/lms/djangoapps/courseware/toggles.py
+++ b/lms/djangoapps/courseware/toggles.py
@@ -148,7 +148,6 @@
# .. toggle_creation_date: 2019-05-16
# .. toggle_expiration_date: None
# .. toggle_tickets: https://github.com/mitodl/edx-platform/issues/123
-# .. toggle_status: unsupported
COURSES_INVITE_ONLY = SettingToggle('COURSES_INVITE_ONLY', default=False)
diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py
index 79a52db8a0b6..1c57b23d9b11 100644
--- a/lms/djangoapps/courseware/views/views.py
+++ b/lms/djangoapps/courseware/views/views.py
@@ -46,7 +46,11 @@
from rest_framework.throttling import UserRateThrottle
from token_utils.api import unpack_token_for
from web_fragments.fragment import Fragment
-from xmodule.course_block import COURSE_VISIBILITY_PUBLIC, COURSE_VISIBILITY_PUBLIC_OUTLINE
+from xmodule.course_block import (
+ COURSE_VISIBILITY_PUBLIC,
+ COURSE_VISIBILITY_PUBLIC_OUTLINE,
+ CATALOG_VISIBILITY_CATALOG_AND_ABOUT,
+)
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
from xmodule.tabs import CourseTabList
@@ -288,7 +292,10 @@ def courses(request):
course_discovery_meanings = getattr(settings, 'COURSE_DISCOVERY_MEANINGS', {})
set_default_filter = ENABLE_COURSE_DISCOVERY_DEFAULT_LANGUAGE_FILTER.is_enabled()
if not settings.FEATURES.get('ENABLE_COURSE_DISCOVERY'):
- courses_list = get_courses(request.user)
+ courses_list = get_courses(
+ request.user,
+ filter_={"catalog_visibility": CATALOG_VISIBILITY_CATALOG_AND_ABOUT},
+ )
if configuration_helpers.get_value("ENABLE_COURSE_SORTING_BY_START_DATE",
settings.FEATURES["ENABLE_COURSE_SORTING_BY_START_DATE"]):
diff --git a/lms/djangoapps/discussion/rest_api/discussions_notifications.py b/lms/djangoapps/discussion/rest_api/discussions_notifications.py
index f65faf7f2a67..b0eb7c89dcab 100644
--- a/lms/djangoapps/discussion/rest_api/discussions_notifications.py
+++ b/lms/djangoapps/discussion/rest_api/discussions_notifications.py
@@ -3,7 +3,7 @@
"""
import re
-from bs4 import BeautifulSoup
+from bs4 import BeautifulSoup, Tag
from django.conf import settings
from django.utils.text import Truncator
@@ -154,6 +154,7 @@ def send_new_comment_notification(self):
"author_name": str(author_name),
"author_pronoun": str(author_pronoun),
"email_content": clean_thread_html_body(self.comment.body),
+ "group_by_id": self.parent_response.id
}
self._send_notification([self.thread.user_id], "new_comment", extra_context=context)
@@ -379,6 +380,30 @@ def remove_html_tags(text):
return re.sub(clean, '', text)
+def strip_empty_tags(soup):
+ """
+ Strip starting and ending empty tags from the soup object
+ """
+ def strip_tag(element, reverse=False):
+ """
+ Checks if element is empty and removes it
+ """
+ if not element.get_text(strip=True):
+ element.extract()
+ return True
+ if isinstance(element, Tag):
+ child_list = element.contents[::-1] if reverse else element.contents
+ for child in child_list:
+ if not strip_tag(child):
+ break
+ return False
+
+ while soup.contents:
+ if not (strip_tag(soup.contents[0]) or strip_tag(soup.contents[-1], reverse=True)):
+ break
+ return soup
+
+
def clean_thread_html_body(html_body):
"""
Get post body with tags removed and limited to 500 characters
@@ -391,7 +416,8 @@ def clean_thread_html_body(html_body):
"video", "track", # Video Tags
"audio", # Audio Tags
"embed", "object", "iframe", # Embedded Content
- "script"
+ "script",
+ "b", "strong", "i", "em", "u", "s", "strike", "del", "ins", "mark", "sub", "sup", # Text Formatting
]
# Remove the specified tags while keeping their content
@@ -399,18 +425,29 @@ def clean_thread_html_body(html_body):
for match in html_body.find_all(tag):
match.unwrap()
+ if not html_body.find():
+ return str(html_body)
+
# Replace tags that are not allowed in email
tags_to_update = [
{"source": "button", "target": "span"},
- {"source": "h1", "target": "h4"},
- {"source": "h2", "target": "h4"},
- {"source": "h3", "target": "h4"},
+ *[
+ {"source": tag, "target": "p"}
+ for tag in ["div", "section", "article", "h1", "h2", "h3", "h4", "h5", "h6"]
+ ],
]
for tag_dict in tags_to_update:
for source_tag in html_body.find_all(tag_dict['source']):
target_tag = html_body.new_tag(tag_dict['target'], **source_tag.attrs)
- if source_tag.string:
- target_tag.string = source_tag.string
- source_tag.replace_with(target_tag)
+ if source_tag.contents:
+ for content in list(source_tag.contents):
+ target_tag.append(content)
+ source_tag.insert_before(target_tag)
+ source_tag.extract()
+
+ for tag in html_body.find_all(True):
+ tag.attrs = {}
+ tag['style'] = 'margin: 0'
+ html_body = strip_empty_tags(html_body)
return str(html_body)
diff --git a/lms/djangoapps/discussion/rest_api/tests/test_discussions_notifications.py b/lms/djangoapps/discussion/rest_api/tests/test_discussions_notifications.py
index d92e1000feb5..9e4a76aa4025 100644
--- a/lms/djangoapps/discussion/rest_api/tests/test_discussions_notifications.py
+++ b/lms/djangoapps/discussion/rest_api/tests/test_discussions_notifications.py
@@ -104,14 +104,14 @@ def test_html_tags_removal(self):
')
result = clean_thread_html_body(html_body)
@@ -132,19 +132,16 @@ def test_truncate_html_body(self):
"""
Test that the clean_thread_html_body function truncates the HTML body to 500 characters
"""
- html_body = """
-
This is a long text that should be truncated to 500 characters.
- """ * 20 # Repeat to exceed 500 characters
-
- result = clean_thread_html_body(html_body)
- self.assertGreaterEqual(500, len(result))
+ html_body = "This is a long text that should be truncated to 500 characters." * 20
+ result = clean_thread_html_body(f"
{html_body}
")
+ self.assertGreaterEqual(525, len(result)) # 500 characters + 25 characters for the HTML tags
def test_no_tags_to_remove(self):
"""
Test that the clean_thread_html_body function does not remove any tags if there are no unwanted tags
"""
html_body = "
This paragraph has no tags to remove.
"
- expected_output = "
This paragraph has no tags to remove.
"
+ expected_output = '
This paragraph has no tags to remove.
'
result = clean_thread_html_body(html_body)
self.assertEqual(result, expected_output)
@@ -169,28 +166,49 @@ def test_only_script_tag(self):
result = clean_thread_html_body(html_body)
self.assertEqual(result.strip(), expected_output)
+ def test_tag_replace(self):
+ """
+ Tests if the clean_thread_html_body function replaces tags
+ """
+ for tag in ["div", "section", "article", "h1", "h2", "h3", "h4", "h5", "h6"]:
+ html_body = f'<{tag}>Text'
+ result = clean_thread_html_body(html_body)
+ self.assertEqual(result, '
Text
')
+
def test_button_tag_replace(self):
"""
Tests that the clean_thread_html_body function replaces the button tag with span tag
"""
- # Tests for button replacement tag with text
html_body = ''
- expected_output = 'Button'
+ expected_output = 'Button'
+ result = clean_thread_html_body(html_body)
+ self.assertEqual(result, expected_output)
+
+ html_body = '
abc
abc
'
+ expected_output = '
abc
'\
+ '
abc
'
result = clean_thread_html_body(html_body)
self.assertEqual(result, expected_output)
- # Tests button tag replacement without text
+ def test_button_tag_removal(self):
+ """
+ Tests button tag with no text is removed if at start or end
+ """
html_body = ''
- expected_output = ''
+ expected_output = ''
result = clean_thread_html_body(html_body)
self.assertEqual(result, expected_output)
- def test_heading_tag_replace(self):
+ def test_attributes_removal_from_tag(self):
+ # Tests for removal of attributes from tags
+ html_body = '
Paragraph
'
+ result = clean_thread_html_body(html_body)
+ self.assertEqual(result, '
Paragraph
')
+
+ def test_strip_empty_tags(self):
"""
- Tests that the clean_thread_html_body function replaces the h1, h2 and h3 tags with h4 tag
+ Tests if the clean_thread_html_body function removes starting and ending empty tags
"""
- for tag in ['h1', 'h2', 'h3']:
- html_body = f'<{tag}>Heading'
- expected_output = '
<%
- if settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS", False):
- certificate_available_date_string = ""
- if course_overview.certificates_display_behavior == CertificatesDisplayBehaviors.END_WITH_DATE and course_overview.certificate_available_date:
- certificate_available_date_string = course_overview.certificate_available_date.strftime('%Y-%m-%dT%H:%M:%S%z')
- elif course_overview.certificates_display_behavior == CertificatesDisplayBehaviors.END and course_overview.end:
- certificate_available_date_string = course_overview.end.strftime('%Y-%m-%dT%H:%M:%S%z')
- else:
+ certificate_available_date_string = ""
+ if course_overview.certificates_display_behavior == CertificatesDisplayBehaviors.END_WITH_DATE and course_overview.certificate_available_date:
certificate_available_date_string = course_overview.certificate_available_date.strftime('%Y-%m-%dT%H:%M:%S%z')
+ elif course_overview.certificates_display_behavior == CertificatesDisplayBehaviors.END and course_overview.end:
+ certificate_available_date_string = course_overview.end.strftime('%Y-%m-%dT%H:%M:%S%z')
container_string = _("Your grade and certificate will be ready after {date}.")
format = 'shortDate'
%>
diff --git a/lms/templates/xblock_v2/xblock_iframe.html b/lms/templates/xblock_v2/xblock_iframe.html
deleted file mode 120000
index 7264c253466d..000000000000
--- a/lms/templates/xblock_v2/xblock_iframe.html
+++ /dev/null
@@ -1 +0,0 @@
-../../../cms/templates/content_libraries/xblock_iframe.html
\ No newline at end of file
diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py
index b5ed1bde78e1..17338f20ab83 100644
--- a/openedx/core/djangoapps/content/search/api.py
+++ b/openedx/core/djangoapps/content/search/api.py
@@ -320,6 +320,7 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None:
Fields.block_id,
Fields.block_type,
Fields.context_key,
+ Fields.usage_key,
Fields.org,
Fields.tags,
Fields.tags + "." + Fields.tags_taxonomy,
diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py
index eabeab9654ca..f520cd14e40f 100644
--- a/openedx/core/djangoapps/content/search/documents.py
+++ b/openedx/core/djangoapps/content/search/documents.py
@@ -267,6 +267,13 @@ def _collections_for_content_object(object_id: UsageKey | LearningContextKey) ->
}
"""
+ result = {
+ Fields.collections: {
+ Fields.collections_display_name: [],
+ Fields.collections_key: [],
+ }
+ }
+
# Gather the collections associated with this object
collections = None
try:
@@ -279,14 +286,8 @@ def _collections_for_content_object(object_id: UsageKey | LearningContextKey) ->
log.warning(f"No component found for {object_id}")
if not collections:
- return {Fields.collections: {}}
+ return result
- result = {
- Fields.collections: {
- Fields.collections_display_name: [],
- Fields.collections_key: [],
- }
- }
for collection in collections:
result[Fields.collections][Fields.collections_display_name].append(collection.title)
result[Fields.collections][Fields.collections_key].append(collection.key)
diff --git a/openedx/core/djangoapps/content/search/handlers.py b/openedx/core/djangoapps/content/search/handlers.py
index 085387d336b1..24add6748d7d 100644
--- a/openedx/core/djangoapps/content/search/handlers.py
+++ b/openedx/core/djangoapps/content/search/handlers.py
@@ -179,13 +179,19 @@ def library_collection_updated_handler(**kwargs) -> None:
log.error("Received null or incorrect data for event")
return
- # Update collection index synchronously to make sure that search index is updated before
- # the frontend invalidates/refetches index.
- # See content_library_updated_handler for more details.
- update_library_collection_index_doc.apply(args=[
- str(library_collection.library_key),
- library_collection.collection_key,
- ])
+ if library_collection.background:
+ update_library_collection_index_doc.delay(
+ str(library_collection.library_key),
+ library_collection.collection_key,
+ )
+ else:
+ # Update collection index synchronously to make sure that search index is updated before
+ # the frontend invalidates/refetches index.
+ # See content_library_updated_handler for more details.
+ update_library_collection_index_doc.apply(args=[
+ str(library_collection.library_key),
+ library_collection.collection_key,
+ ])
@receiver(CONTENT_OBJECT_ASSOCIATIONS_CHANGED)
diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py
index 4c6227af309f..c89d84490e96 100644
--- a/openedx/core/djangoapps/content/search/tests/test_api.py
+++ b/openedx/core/djangoapps/content/search/tests/test_api.py
@@ -219,10 +219,10 @@ def test_reindex_meilisearch(self, mock_meilisearch):
doc_vertical["tags"] = {}
doc_problem1 = copy.deepcopy(self.doc_problem1)
doc_problem1["tags"] = {}
- doc_problem1["collections"] = {}
+ doc_problem1["collections"] = {'display_name': [], 'key': []}
doc_problem2 = copy.deepcopy(self.doc_problem2)
doc_problem2["tags"] = {}
- doc_problem2["collections"] = {}
+ doc_problem2["collections"] = {'display_name': [], 'key': []}
doc_collection = copy.deepcopy(self.collection_dict)
doc_collection["tags"] = {}
@@ -263,7 +263,7 @@ def test_reindex_meilisearch_library_block_error(self, mock_meilisearch):
doc_vertical["tags"] = {}
doc_problem2 = copy.deepcopy(self.doc_problem2)
doc_problem2["tags"] = {}
- doc_problem2["collections"] = {}
+ doc_problem2["collections"] = {'display_name': [], 'key': []}
orig_from_component = library_api.LibraryXBlockMetadata.from_component
@@ -662,7 +662,7 @@ def test_delete_collection(self, mock_meilisearch):
doc_problem_without_collection = {
"id": self.doc_problem1["id"],
- "collections": {},
+ "collections": {'display_name': [], 'key': []},
}
# Should delete the collection document
diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py
index b9f3779af539..90b73ef7f474 100644
--- a/openedx/core/djangoapps/content_libraries/api.py
+++ b/openedx/core/djangoapps/content_libraries/api.py
@@ -56,6 +56,7 @@
import base64
import hashlib
import logging
+import mimetypes
import attr
import requests
@@ -68,6 +69,7 @@
from django.db.models import Q, QuerySet
from django.utils.translation import gettext as _
from edx_rest_api_client.client import OAuthAPIClient
+from django.urls import reverse
from lxml import etree
from opaque_keys.edx.keys import BlockTypeKey, UsageKey, UsageKeyV2
from opaque_keys.edx.locator import (
@@ -76,10 +78,10 @@
LibraryLocator as LibraryLocatorV1,
LibraryCollectionLocator,
)
-from opaque_keys import InvalidKeyError
from openedx_events.content_authoring.data import (
ContentLibraryData,
LibraryBlockData,
+ LibraryCollectionData,
)
from openedx_events.content_authoring.signals import (
CONTENT_LIBRARY_CREATED,
@@ -88,6 +90,7 @@
LIBRARY_BLOCK_CREATED,
LIBRARY_BLOCK_DELETED,
LIBRARY_BLOCK_UPDATED,
+ LIBRARY_COLLECTION_UPDATED,
)
from openedx_learning.api import authoring as authoring_api
from openedx_learning.api.authoring_models import Collection, Component, MediaType, LearningPackage, PublishableEntity
@@ -95,12 +98,13 @@
from xblock.core import XBlock
from xblock.exceptions import XBlockNotFoundError
-from openedx.core.djangoapps.xblock.api import get_component_from_usage_key, xblock_type_display_name
+from openedx.core.djangoapps.xblock.api import (
+ get_component_from_usage_key,
+ get_xblock_app_config,
+ xblock_type_display_name,
+)
from openedx.core.lib.xblock_serializer.api import serialize_modulestore_block_for_learning_core
-from xmodule.library_root_xblock import LibraryRoot as LibraryRootV1
-from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
-from xmodule.modulestore.exceptions import ItemNotFoundError
from . import permissions, tasks
from .constants import ALL_RIGHTS_RESERVED, COMPLEX
@@ -204,6 +208,15 @@ class ContentLibraryPermissionEntry:
access_level = attr.ib(AccessLevel.NO_ACCESS)
+@attr.s
+class CollectionMetadata:
+ """
+ Class to represent collection metadata in a content library.
+ """
+ key = attr.ib(type=str)
+ title = attr.ib(type=str)
+
+
@attr.s
class LibraryXBlockMetadata:
"""
@@ -219,9 +232,10 @@ class LibraryXBlockMetadata:
published_by = attr.ib("")
has_unpublished_changes = attr.ib(False)
created = attr.ib(default=None, type=datetime)
+ collections = attr.ib(type=list[CollectionMetadata], factory=list)
@classmethod
- def from_component(cls, library_key, component):
+ def from_component(cls, library_key, component, associated_collections=None):
"""
Construct a LibraryXBlockMetadata from a Component object.
"""
@@ -248,6 +262,7 @@ def from_component(cls, library_key, component):
last_draft_created=last_draft_created,
last_draft_created_by=last_draft_created_by,
has_unpublished_changes=component.versioning.has_unpublished_changes,
+ collections=associated_collections or [],
)
@@ -408,8 +423,8 @@ def get_library(library_key):
# updated version of content that a course could pull in. But more recently,
# we've decided to do those version references at the level of the
# individual blocks being used, since a Learning Core backed library is
- # intended to be used for many LibraryContentBlocks and not 1:1 like v1
- # libraries. The top level version stays for now because LibraryContentBlock
+ # intended to be referenced in multiple course locations and not 1:1 like v1
+ # libraries. The top level version stays for now because LegacyLibraryContentBlock
# uses it, but that should hopefully change before the Redwood release.
version = 0 if last_publish_log is None else last_publish_log.pk
published_by = None
@@ -690,7 +705,7 @@ def get_library_components(library_key, text_search=None, block_types=None) -> Q
return components
-def get_library_block(usage_key) -> LibraryXBlockMetadata:
+def get_library_block(usage_key, include_collections=False) -> LibraryXBlockMetadata:
"""
Get metadata about (the draft version of) one specific XBlock in a library.
@@ -713,20 +728,30 @@ def get_library_block(usage_key) -> LibraryXBlockMetadata:
if not draft_version:
raise ContentLibraryBlockNotFound(usage_key)
+ if include_collections:
+ associated_collections = authoring_api.get_entity_collections(
+ component.learning_package_id,
+ component.key,
+ ).values('key', 'title')
+ else:
+ associated_collections = None
xblock_metadata = LibraryXBlockMetadata.from_component(
library_key=usage_key.context_key,
component=component,
+ associated_collections=associated_collections,
)
return xblock_metadata
-def set_library_block_olx(usage_key, new_olx_str):
+def set_library_block_olx(usage_key, new_olx_str) -> int:
"""
Replace the OLX source of the given XBlock.
This is only meant for use by developers or API client applications, as
very little validation is done and this can easily result in a broken XBlock
that won't load.
+
+ Returns the version number of the newly created ComponentVersion.
"""
# because this old pylint can't understand attr.ib() objects, pylint: disable=no-member
assert isinstance(usage_key, LibraryUsageLocatorV2)
@@ -763,7 +788,7 @@ def set_library_block_olx(usage_key, new_olx_str):
text=new_olx_str,
created=now,
)
- authoring_api.create_next_version(
+ new_component_version = authoring_api.create_next_component_version(
component.pk,
title=new_title,
content_to_replace={
@@ -779,6 +804,8 @@ def set_library_block_olx(usage_key, new_olx_str):
)
)
+ return new_component_version.version_num
+
def library_component_usage_key(
library_key: LibraryLocatorV2,
@@ -1001,18 +1028,48 @@ def get_library_block_static_asset_files(usage_key) -> list[LibraryXBlockStaticF
Returns a list of LibraryXBlockStaticFile objects, sorted by path.
- TODO: This is not yet implemented for Learning Core backed libraries.
TODO: Should this be in the general XBlock API rather than the libraries API?
"""
- return []
+ component = get_component_from_usage_key(usage_key)
+ component_version = component.versioning.draft
+
+ # If there is no Draft version, then this was soft-deleted
+ if component_version is None:
+ return []
+
+ # cvc = the ComponentVersionContent through table
+ cvc_set = (
+ component_version
+ .componentversioncontent_set
+ .filter(content__has_file=True)
+ .order_by('key')
+ .select_related('content')
+ )
+ site_root_url = get_xblock_app_config().get_site_root_url()
-def add_library_block_static_asset_file(usage_key, file_name, file_content) -> LibraryXBlockStaticFile:
+ return [
+ LibraryXBlockStaticFile(
+ path=cvc.key,
+ size=cvc.content.size,
+ url=site_root_url + reverse(
+ 'content_libraries:library-assets',
+ kwargs={
+ 'component_version_uuid': component_version.uuid,
+ 'asset_path': cvc.key,
+ }
+ ),
+ )
+ for cvc in cvc_set
+ ]
+
+
+def add_library_block_static_asset_file(usage_key, file_path, file_content, user=None) -> LibraryXBlockStaticFile:
"""
Upload a static asset file into the library, to be associated with the
specified XBlock. Will silently overwrite an existing file of the same name.
- file_name should be a name like "doc.pdf". It may optionally contain slashes
+ file_path should be a name like "doc.pdf". It may optionally contain slashes
like 'en/doc.pdf'
file_content should be a binary string.
@@ -1024,10 +1081,67 @@ def add_library_block_static_asset_file(usage_key, file_name, file_content) -> L
video_block = UsageKey.from_string("lb:VideoTeam:python-intro:video:1")
add_library_block_static_asset_file(video_block, "subtitles-en.srt", subtitles.encode('utf-8'))
"""
- raise NotImplementedError("Static assets not yet implemented for Learning Core")
+ # File path validations copied over from v1 library logic. This can't really
+ # hurt us inside our system because we never use these paths in an actual
+ # file system–they're just string keys that point to hash-named data files
+ # in a common library (learning package) level directory. But it might
+ # become a security issue during import/export serialization.
+ if file_path != file_path.strip().strip('/'):
+ raise InvalidNameError("file_path cannot start/end with / or whitespace.")
+ if '//' in file_path or '..' in file_path:
+ raise InvalidNameError("Invalid sequence (// or ..) in file_path.")
+ component = get_component_from_usage_key(usage_key)
+
+ media_type_str, _encoding = mimetypes.guess_type(file_path)
+ # We use "application/octet-stream" as a generic fallback media type, per
+ # RFC 2046: https://datatracker.ietf.org/doc/html/rfc2046
+ # TODO: This probably makes sense to push down to openedx-learning?
+ media_type_str = media_type_str or "application/octet-stream"
+
+ now = datetime.now(tz=timezone.utc)
-def delete_library_block_static_asset_file(usage_key, file_name):
+ with transaction.atomic():
+ media_type = authoring_api.get_or_create_media_type(media_type_str)
+ content = authoring_api.get_or_create_file_content(
+ component.publishable_entity.learning_package.id,
+ media_type.id,
+ data=file_content,
+ created=now,
+ )
+ component_version = authoring_api.create_next_component_version(
+ component.pk,
+ content_to_replace={file_path: content.id},
+ created=now,
+ created_by=user.id if user else None,
+ )
+ transaction.on_commit(
+ lambda: LIBRARY_BLOCK_UPDATED.send_event(
+ library_block=LibraryBlockData(
+ library_key=usage_key.context_key,
+ usage_key=usage_key,
+ )
+ )
+ )
+
+ # Now figure out the URL for the newly created asset...
+ site_root_url = get_xblock_app_config().get_site_root_url()
+ local_path = reverse(
+ 'content_libraries:library-assets',
+ kwargs={
+ 'component_version_uuid': component_version.uuid,
+ 'asset_path': file_path,
+ }
+ )
+
+ return LibraryXBlockStaticFile(
+ path=file_path,
+ url=site_root_url + local_path,
+ size=content.size,
+ )
+
+
+def delete_library_block_static_asset_file(usage_key, file_path, user=None):
"""
Delete a static asset file from the library.
@@ -1037,7 +1151,24 @@ def delete_library_block_static_asset_file(usage_key, file_name):
video_block = UsageKey.from_string("lb:VideoTeam:python-intro:video:1")
delete_library_block_static_asset_file(video_block, "subtitles-en.srt")
"""
- raise NotImplementedError("Static assets not yet implemented for Learning Core")
+ component = get_component_from_usage_key(usage_key)
+ now = datetime.now(tz=timezone.utc)
+
+ with transaction.atomic():
+ component_version = authoring_api.create_next_component_version(
+ component.pk,
+ content_to_replace={file_path: None},
+ created=now,
+ created_by=user.id if user else None,
+ )
+ transaction.on_commit(
+ lambda: LIBRARY_BLOCK_UPDATED.send_event(
+ library_block=LibraryBlockData(
+ library_key=usage_key.context_key,
+ usage_key=usage_key,
+ )
+ )
+ )
def get_allowed_block_types(library_key): # pylint: disable=unused-argument
@@ -1235,6 +1366,60 @@ def update_library_collection_components(
return collection
+def set_library_component_collections(
+ library_key: LibraryLocatorV2,
+ component: Component,
+ *,
+ collection_keys: list[str],
+ created_by: int | None = None,
+ # As an optimization, callers may pass in a pre-fetched ContentLibrary instance
+ content_library: ContentLibrary | None = None,
+) -> Component:
+ """
+ It Associates the component with collections for the given collection keys.
+
+ Only collections in queryset are associated with component, all previous component-collections
+ associations are removed.
+
+ If you've already fetched the ContentLibrary, pass it in to avoid refetching.
+
+ Raises:
+ * ContentLibraryCollectionNotFound if any of the given collection_keys don't match Collections in the given library.
+
+ Returns the updated Component.
+ """
+ if not content_library:
+ content_library = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined]
+ assert content_library
+ assert content_library.learning_package_id
+ assert content_library.library_key == library_key
+
+ # Note: Component.key matches its PublishableEntity.key
+ collection_qs = authoring_api.get_collections(content_library.learning_package_id).filter(
+ key__in=collection_keys
+ )
+
+ affected_collections = authoring_api.set_collections(
+ content_library.learning_package_id,
+ component,
+ collection_qs,
+ created_by=created_by,
+ )
+
+ # For each collection, trigger LIBRARY_COLLECTION_UPDATED signal and set background=True to trigger
+ # collection indexing asynchronously.
+ for collection in affected_collections:
+ LIBRARY_COLLECTION_UPDATED.send_event(
+ library_collection=LibraryCollectionData(
+ library_key=library_key,
+ collection_key=collection.key,
+ background=True,
+ )
+ )
+
+ return component
+
+
def get_library_collection_usage_key(
library_key: LibraryLocatorV2,
collection_key: str,
@@ -1265,77 +1450,6 @@ def get_library_collection_from_usage_key(
raise ContentLibraryCollectionNotFound from exc
-# V1/V2 Compatibility Helpers
-# (Should be removed as part of
-# https://github.com/openedx/edx-platform/issues/32457)
-# ======================================================
-
-def get_v1_or_v2_library(
- library_id: str | LibraryLocatorV1 | LibraryLocatorV2,
- version: str | int | None,
-) -> LibraryRootV1 | ContentLibraryMetadata | None:
- """
- Fetch either a V1 or V2 content library from a V1/V2 key (or key string) and version.
-
- V1 library versions are Mongo ObjectID strings.
- V2 library versions can be positive ints, or strings of positive ints.
- Passing version=None will return the latest version the library.
-
- Returns None if not found.
- If key is invalid, raises InvalidKeyError.
- For V1, if key has a version, it is ignored in favor of `version`.
- For V2, if version is provided but it isn't an int or parseable to one, we raise a ValueError.
-
- Examples:
- * get_v1_or_v2_library("library-v1:ProblemX+PR0B", None) ->
- * get_v1_or_v2_library("library-v1:ProblemX+PR0B", "65ff...") ->
- * get_v1_or_v2_library("lib:RG:rg-1", None) ->
- * get_v1_or_v2_library("lib:RG:rg-1", "36") ->
- * get_v1_or_v2_library("lib:RG:rg-1", "xyz") ->
- * get_v1_or_v2_library("notakey", "xyz") ->
-
- If you just want to get a V2 library, use `get_library` instead.
- """
- library_key: LibraryLocatorV1 | LibraryLocatorV2
- if isinstance(library_id, str):
- try:
- library_key = LibraryLocatorV1.from_string(library_id)
- except InvalidKeyError:
- library_key = LibraryLocatorV2.from_string(library_id)
- else:
- library_key = library_id
- if isinstance(library_key, LibraryLocatorV2):
- v2_version: int | None
- if version:
- v2_version = int(version)
- else:
- v2_version = None
- try:
- library = get_library(library_key)
- if v2_version is not None and library.version != v2_version:
- raise NotImplementedError(
- f"Tried to load version {v2_version} of learning_core-based library {library_key}. "
- f"Currently, only the latest version ({library.version}) may be loaded. "
- "This is a known issue. "
- "It will be fixed before the production release of learning_core-based (V2) content libraries. "
- )
- return library
- except ContentLibrary.DoesNotExist:
- return None
- elif isinstance(library_key, LibraryLocatorV1):
- v1_version: str | None
- if version:
- v1_version = str(version)
- else:
- v1_version = None
- store = modulestore()
- library_key = library_key.for_branch(ModuleStoreEnum.BranchName.library).for_version(v1_version)
- try:
- return store.get_library(library_key, remove_version=False, remove_branch=False, head_validation=False)
- except ItemNotFoundError:
- return None
-
-
# Import from Courseware
# ======================
diff --git a/openedx/core/djangoapps/content_libraries/library_context.py b/openedx/core/djangoapps/content_libraries/library_context.py
index 6ff426e73560..4bda10eb12fa 100644
--- a/openedx/core/djangoapps/content_libraries/library_context.py
+++ b/openedx/core/djangoapps/content_libraries/library_context.py
@@ -1,19 +1,21 @@
"""
Definition of "Library" as a learning context.
"""
-
import logging
from django.core.exceptions import PermissionDenied
+from rest_framework.exceptions import NotFound
from openedx_events.content_authoring.data import LibraryBlockData
from openedx_events.content_authoring.signals import LIBRARY_BLOCK_UPDATED
+from opaque_keys.edx.keys import UsageKeyV2
+from opaque_keys.edx.locator import LibraryUsageLocatorV2, LibraryLocatorV2
+from openedx_learning.api import authoring as authoring_api
from openedx.core.djangoapps.content_libraries import api, permissions
from openedx.core.djangoapps.content_libraries.models import ContentLibrary
from openedx.core.djangoapps.xblock.api import LearningContext
-
-from openedx_learning.api import authoring as authoring_api
+from openedx.core.types import User as UserType
log = logging.getLogger(__name__)
@@ -30,47 +32,51 @@ def __init__(self, **kwargs):
super().__init__(**kwargs)
self.use_draft = kwargs.get('use_draft', None)
- def can_edit_block(self, user, usage_key):
+ def can_edit_block(self, user: UserType, usage_key: UsageKeyV2) -> bool:
"""
- Does the specified usage key exist in its context, and if so, does the
- specified user have permission to edit it (make changes to the authored
- data store)?
-
- user: a Django User object (may be an AnonymousUser)
+ Assuming a block with the specified ID (usage_key) exists, does the
+ specified user have permission to edit it (make changes to the
+ fields / authored data store)?
- usage_key: the UsageKeyV2 subclass used for this learning context
+ May raise ContentLibraryNotFound if the library does not exist.
+ """
+ assert isinstance(usage_key, LibraryUsageLocatorV2)
+ return self._check_perm(user, usage_key.lib_key, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY)
- Must return a boolean.
+ def can_view_block_for_editing(self, user: UserType, usage_key: UsageKeyV2) -> bool:
"""
- try:
- api.require_permission_for_library_key(usage_key.lib_key, user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY)
- except (PermissionDenied, api.ContentLibraryNotFound):
- return False
+ Assuming a block with the specified ID (usage_key) exists, does the
+ specified user have permission to view its fields and OLX details (but
+ not necessarily to make changes to it)?
- return self.block_exists(usage_key)
+ May raise ContentLibraryNotFound if the library does not exist.
+ """
+ assert isinstance(usage_key, LibraryUsageLocatorV2)
+ return self._check_perm(user, usage_key.lib_key, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY)
- def can_view_block(self, user, usage_key):
+ def can_view_block(self, user: UserType, usage_key: UsageKeyV2) -> bool:
"""
Does the specified usage key exist in its context, and if so, does the
specified user have permission to view it and interact with it (call
handlers, save user state, etc.)?
- user: a Django User object (may be an AnonymousUser)
-
- usage_key: the UsageKeyV2 subclass used for this learning context
-
- Must return a boolean.
+ May raise ContentLibraryNotFound if the library does not exist.
"""
+ assert isinstance(usage_key, LibraryUsageLocatorV2)
+ return self._check_perm(user, usage_key.lib_key, permissions.CAN_LEARN_FROM_THIS_CONTENT_LIBRARY)
+
+ def _check_perm(self, user: UserType, lib_key: LibraryLocatorV2, perm) -> bool:
+ """ Helper method to check a permission for the various can_ methods"""
try:
- api.require_permission_for_library_key(
- usage_key.lib_key, user, permissions.CAN_LEARN_FROM_THIS_CONTENT_LIBRARY,
- )
- except (PermissionDenied, api.ContentLibraryNotFound):
+ api.require_permission_for_library_key(lib_key, user, perm)
+ return True
+ except PermissionDenied:
return False
+ except api.ContentLibraryNotFound as exc:
+ # A 404 is probably what you want in this case, not a 500 error, so do that by default.
+ raise NotFound(f"Content Library '{lib_key}' does not exist") from exc
- return self.block_exists(usage_key)
-
- def block_exists(self, usage_key):
+ def block_exists(self, usage_key: LibraryUsageLocatorV2):
"""
Does the block for this usage_key exist in this Library?
@@ -82,7 +88,7 @@ def block_exists(self, usage_key):
version of it.
"""
try:
- content_lib = ContentLibrary.objects.get_by_key(usage_key.context_key)
+ content_lib = ContentLibrary.objects.get_by_key(usage_key.context_key) # type: ignore[attr-defined]
except ContentLibrary.DoesNotExist:
return False
@@ -97,12 +103,11 @@ def block_exists(self, usage_key):
local_key=usage_key.block_id,
)
- def send_block_updated_event(self, usage_key):
+ def send_block_updated_event(self, usage_key: UsageKeyV2):
"""
Send a "block updated" event for the library block with the given usage_key.
-
- usage_key: the UsageKeyV2 subclass used for this learning context
"""
+ assert isinstance(usage_key, LibraryUsageLocatorV2)
LIBRARY_BLOCK_UPDATED.send_event(
library_block=LibraryBlockData(
library_key=usage_key.lib_key,
diff --git a/openedx/core/djangoapps/content_libraries/permissions.py b/openedx/core/djangoapps/content_libraries/permissions.py
index c7da012c9fb4..17671b5659f8 100644
--- a/openedx/core/djangoapps/content_libraries/permissions.py
+++ b/openedx/core/djangoapps/content_libraries/permissions.py
@@ -2,7 +2,8 @@
Permissions for Content Libraries (v2, Learning-Core-based)
"""
from bridgekeeper import perms, rules
-from bridgekeeper.rules import Attribute, ManyRelation, Relation, in_current_groups
+from bridgekeeper.rules import Attribute, ManyRelation, Relation, blanket_rule, in_current_groups
+from django.conf import settings
from openedx.core.djangoapps.content_libraries.models import ContentLibraryPermission
@@ -41,6 +42,12 @@
)
+# Are we in Studio? (Is there a better or more contextual way to define this, e.g. get from learning context?)
+@blanket_rule
+def is_studio_request(_):
+ return settings.SERVICE_VARIANT == "cms"
+
+
########################### Permissions ###########################
# Is the user allowed to view XBlocks from the specified content library
@@ -51,10 +58,12 @@
perms[CAN_LEARN_FROM_THIS_CONTENT_LIBRARY] = (
# Global staff can learn from any library:
is_global_staff |
- # Regular users can learn if the library allows public learning:
+ # Regular and even anonymous users can learn if the library allows public learning:
Attribute('allow_public_learning', True) |
# Users/groups who are explicitly granted permission can learn from the library:
- (is_user_active & has_explicit_read_permission_for_library)
+ (is_user_active & has_explicit_read_permission_for_library) |
+ # Or, in Studio (but not the LMS) any users can access libraries with "public read" permissions:
+ (is_studio_request & is_user_active & Attribute('allow_public_read', True))
)
# Is the user allowed to create content libraries?
diff --git a/openedx/core/djangoapps/content_libraries/serializers.py b/openedx/core/djangoapps/content_libraries/serializers.py
index 51ba55cd6b48..b19d27bed3bb 100644
--- a/openedx/core/djangoapps/content_libraries/serializers.py
+++ b/openedx/core/djangoapps/content_libraries/serializers.py
@@ -134,6 +134,14 @@ class ContentLibraryFilterSerializer(BaseFilterSerializer):
type = serializers.ChoiceField(choices=LIBRARY_TYPES, default=None, required=False)
+class CollectionMetadataSerializer(serializers.Serializer):
+ """
+ Serializer for CollectionMetadata
+ """
+ key = serializers.CharField()
+ title = serializers.CharField()
+
+
class LibraryXBlockMetadataSerializer(serializers.Serializer):
"""
Serializer for LibraryXBlockMetadata
@@ -161,6 +169,8 @@ class LibraryXBlockMetadataSerializer(serializers.Serializer):
slug = serializers.CharField(write_only=True)
tags_count = serializers.IntegerField(read_only=True)
+ collections = CollectionMetadataSerializer(many=True, required=False)
+
class LibraryXBlockTypeSerializer(serializers.Serializer):
"""
@@ -207,6 +217,7 @@ class LibraryXBlockOlxSerializer(serializers.Serializer):
Serializer for representing an XBlock's OLX
"""
olx = serializers.CharField()
+ version_num = serializers.IntegerField(read_only=True, required=False)
class LibraryXBlockStaticFileSerializer(serializers.Serializer):
@@ -305,3 +316,11 @@ class ContentLibraryCollectionComponentsUpdateSerializer(serializers.Serializer)
"""
usage_keys = serializers.ListField(child=UsageKeyV2Serializer(), allow_empty=False)
+
+
+class ContentLibraryComponentCollectionsUpdateSerializer(serializers.Serializer):
+ """
+ Serializer for adding/removing Collections to/from a Component.
+ """
+
+ collection_keys = serializers.ListField(child=serializers.CharField(), allow_empty=True)
diff --git a/openedx/core/djangoapps/content_libraries/signal_handlers.py b/openedx/core/djangoapps/content_libraries/signal_handlers.py
index fedee045a9f6..58f45d218e95 100644
--- a/openedx/core/djangoapps/content_libraries/signal_handlers.py
+++ b/openedx/core/djangoapps/content_libraries/signal_handlers.py
@@ -20,8 +20,8 @@
LIBRARY_COLLECTION_DELETED,
LIBRARY_COLLECTION_UPDATED,
)
-from openedx_learning.api.authoring import get_collection_components, get_component, get_components
-from openedx_learning.api.authoring_models import Collection, CollectionPublishableEntity, Component
+from openedx_learning.api.authoring import get_component, get_components
+from openedx_learning.api.authoring_models import Collection, CollectionPublishableEntity, Component, PublishableEntity
from lms.djangoapps.grades.api import signals as grades_signals
@@ -167,9 +167,11 @@ def library_collection_entity_deleted(sender, instance, **kwargs):
"""
Sends a CONTENT_OBJECT_ASSOCIATIONS_CHANGED event for components removed from a collection.
"""
- # Component.pk matches its entity.pk
- component = get_component(instance.entity_id)
- _library_collection_component_changed(component)
+ # Only trigger component updates if CollectionPublishableEntity was cascade deleted due to deletion of a collection.
+ if isinstance(kwargs.get('origin'), Collection):
+ # Component.pk matches its entity.pk
+ component = get_component(instance.entity_id)
+ _library_collection_component_changed(component)
@receiver(m2m_changed, sender=CollectionPublishableEntity, dispatch_uid="library_collection_entities_changed")
@@ -177,9 +179,6 @@ def library_collection_entities_changed(sender, instance, action, pk_set, **kwar
"""
Sends a CONTENT_OBJECT_ASSOCIATIONS_CHANGED event for components added/removed/cleared from a collection.
"""
- if not isinstance(instance, Collection):
- return
-
if action not in ["post_add", "post_remove", "post_clear"]:
return
@@ -191,18 +190,16 @@ def library_collection_entities_changed(sender, instance, action, pk_set, **kwar
log.error("{instance} is not associated with a content library.")
return
+ if isinstance(instance, PublishableEntity):
+ _library_collection_component_changed(instance.component, library.library_key)
+ return
+
+ # When action=="post_clear", pk_set==None
+ # Since the collection instance now has an empty entities set,
+ # we don't know which ones were removed, so we need to update associations for all library components.
+ components = get_components(instance.learning_package_id)
if pk_set:
- components = get_collection_components(
- instance.learning_package_id,
- instance.key,
- ).filter(pk__in=pk_set)
- else:
- # When action=="post_clear", pk_set==None
- # Since the collection instance now has an empty entities set,
- # we don't know which ones were removed, so we need to update associations for all library components.
- components = get_components(
- instance.learning_package_id,
- )
+ components = components.filter(pk__in=pk_set)
for component in components.all():
_library_collection_component_changed(component, library.library_key)
diff --git a/openedx/core/djangoapps/content_libraries/tasks.py b/openedx/core/djangoapps/content_libraries/tasks.py
index 9f4f7aaaf7dc..f56b4adfe313 100644
--- a/openedx/core/djangoapps/content_libraries/tasks.py
+++ b/openedx/core/djangoapps/content_libraries/tasks.py
@@ -21,33 +21,20 @@
from celery import shared_task
from celery_utils.logged_task import LoggedTask
from celery.utils.log import get_task_logger
-from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user
-from django.core.exceptions import PermissionDenied
from edx_django_utils.monitoring import set_code_owner_attribute, set_code_owner_attribute_from_module
-from opaque_keys.edx.keys import UsageKey
-from opaque_keys.edx.locator import (
- BlockUsageLocator,
- LibraryLocatorV2,
- LibraryUsageLocatorV2,
- LibraryLocator as LibraryLocatorV1
-)
from user_tasks.tasks import UserTask, UserTaskStatus
from xblock.fields import Scope
-from common.djangoapps.student.auth import has_studio_write_access
from opaque_keys.edx.keys import CourseKey
-from openedx.core.djangoapps.content_libraries import api as library_api
-from openedx.core.djangoapps.xblock.api import load_block
+from opaque_keys.edx.locator import BlockUsageLocator
from openedx.core.lib import ensure_cms
from xmodule.capa_block import ProblemBlock
-from xmodule.library_content_block import ANY_CAPA_TYPE_VALUE, LibraryContentBlock
-from xmodule.library_root_xblock import LibraryRoot as LibraryRootV1
+from xmodule.library_content_block import ANY_CAPA_TYPE_VALUE, LegacyLibraryContentBlock
+from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.modulestore.mixed import MixedModuleStore
-from xmodule.modulestore.split_mongo import BlockKey
-from xmodule.util.keys import derive_key
from . import api
from .models import ContentLibraryBlockImportTask
@@ -84,77 +71,6 @@ def on_progress(block_key, block_num, block_count, exception=None):
)
-def _import_block(store, user_id, source_block, dest_parent_key):
- """
- Recursively import a learning core block and its children.`
- """
- def generate_block_key(source_key, dest_parent_key):
- """
- Deterministically generate an ID for the new block and return the key.
- Keys are generated such that they appear identical to a v1 library with
- the same input block_id, library name, library organization, and parent block using derive_key
- """
- if not isinstance(source_key.lib_key, LibraryLocatorV2):
- raise TypeError(f"Expected source library key of type LibraryLocatorV2, got {source_key.lib_key} instead.")
- source_key_as_v1_course_key = LibraryLocatorV1(
- org=source_key.lib_key.org,
- library=source_key.lib_key.slug,
- branch='library'
- )
- derived_block_key = derive_key(
- source=source_key_as_v1_course_key.make_usage_key(source_key.block_type, source_key.block_id),
- dest_parent=BlockKey(dest_parent_key.block_type, dest_parent_key.block_id),
- )
- return dest_parent_key.context_key.make_usage_key(*derived_block_key)
-
- source_key = source_block.scope_ids.usage_id
- new_block_key = generate_block_key(source_key, dest_parent_key)
- try:
- new_block = store.get_item(new_block_key)
- if new_block.parent.block_id != dest_parent_key.block_id:
- raise ValueError(
- "Expected existing block {} to be a child of {} but instead it's a child of {}".format(
- new_block_key, dest_parent_key, new_block.parent,
- )
- )
- except ItemNotFoundError:
- new_block = store.create_child(
- user_id,
- dest_parent_key,
- source_key.block_type,
- block_id=new_block_key.block_id,
- )
-
- # Prepare a list of this block's static assets; any assets that are referenced as /static/{path} (the
- # recommended way for referencing them) will stop working, and so we rewrite the url when importing.
- # Copying assets not advised because modulestore doesn't namespace assets to each block like learning core, which
- # might cause conflicts when the same filename is used across imported blocks.
- if isinstance(source_key, LibraryUsageLocatorV2):
- all_assets = library_api.get_library_block_static_asset_files(source_key)
- else:
- all_assets = []
-
- for field_name, field in source_block.fields.items():
- if field.scope not in (Scope.settings, Scope.content):
- continue # Only copy authored field data
- if field.is_set_on(source_block) or field.is_set_on(new_block):
- field_value = getattr(source_block, field_name)
- setattr(new_block, field_name, field_value)
- new_block.save()
- store.update_item(new_block, user_id)
-
- if new_block.has_children:
- # Delete existing children in the new block, which can be reimported again if they still exist in the
- # source library
- for existing_child_key in new_block.children:
- store.delete_item(existing_child_key, user_id)
- # Now import the children
- for child in source_block.get_children():
- _import_block(store, user_id, child, new_block_key)
-
- return new_block_key
-
-
def _filter_child(store, usage_key, capa_type):
"""
Return whether this block is both a problem and has a `capa_type` which is included in the filter.
@@ -172,49 +88,6 @@ def _problem_type_filter(store, library, capa_type):
return [key for key in library.children if _filter_child(store, key, capa_type)]
-def _import_from_learning_core(user_id, store, dest_block, source_block_ids):
- """
- Imports a block from a learning-core-based learning context (usually a
- content library) into modulestore, as a new child of dest_block.
- Any existing children of dest_block are replaced.
- """
- dest_key = dest_block.scope_ids.usage_id
- if not isinstance(dest_key, BlockUsageLocator):
- raise TypeError(f"Destination {dest_key} should be a modulestore course.")
- if user_id is None:
- raise ValueError("Cannot check user permissions - LibraryTools user_id is None")
-
- if len(set(source_block_ids)) != len(source_block_ids):
- # We don't support importing the exact same block twice because it would break the way we generate new IDs
- # for each block and then overwrite existing copies of blocks when re-importing the same blocks.
- raise ValueError("One or more library component IDs is a duplicate.")
-
- dest_course_key = dest_key.context_key
- user = User.objects.get(id=user_id)
- if not has_studio_write_access(user, dest_course_key):
- raise PermissionDenied()
-
- # Read the source block; this will also confirm that user has permission to read it.
- # (This could be slow and use lots of memory, except for the fact that LibraryContentBlock which calls this
- # should be limiting the number of blocks to a reasonable limit. We load them all now instead of one at a
- # time in order to raise any errors before we start actually copying blocks over.)
- orig_blocks = [load_block(UsageKey.from_string(key), user) for key in source_block_ids]
-
- with store.bulk_operations(dest_course_key):
- child_ids_updated = set()
-
- for block in orig_blocks:
- new_block_id = _import_block(store, user_id, block, dest_key)
- child_ids_updated.add(new_block_id)
-
- # Remove any existing children that are no longer used
- for old_child_id in set(dest_block.children) - child_ids_updated:
- store.delete_item(old_child_id, user_id)
- # If this was called from a handler, it will save dest_block at the end, so we must update
- # dest_block.children to avoid it saving the old value of children and deleting the new ones.
- dest_block.children = store.get_item(dest_key).children
-
-
class LibrarySyncChildrenTask(UserTask): # pylint: disable=abstract-method
"""
Base class for tasks which operate upon library_content children.
@@ -244,7 +117,7 @@ def sync_from_library(
self: LibrarySyncChildrenTask,
user_id: int,
dest_block_id: str,
- library_version: str | int | None,
+ library_version: str | None,
) -> None:
"""
Celery task to update the children of the library_content block at `dest_block_id`.
@@ -300,8 +173,8 @@ def _sync_children(
task: LibrarySyncChildrenTask,
store: MixedModuleStore,
user_id: int,
- dest_block: LibraryContentBlock,
- library_version: int | str | None,
+ dest_block: LegacyLibraryContentBlock,
+ library_version: str | None,
) -> None:
"""
Implementation helper for `sync_from_library` and `duplicate_children` Celery tasks.
@@ -309,41 +182,29 @@ def _sync_children(
Can update children with a specific library `library_version`, or latest (`library_version=None`).
"""
source_blocks = []
- library_key = dest_block.source_library_key
- filter_children = (dest_block.capa_type != ANY_CAPA_TYPE_VALUE)
- library = library_api.get_v1_or_v2_library(library_key, version=library_version)
- if not library:
+ library_key = dest_block.source_library_key.for_branch(
+ ModuleStoreEnum.BranchName.library
+ ).for_version(library_version)
+ try:
+ library = store.get_library(library_key, remove_version=False, remove_branch=False, head_validation=False)
+ except ItemNotFoundError:
task.status.fail(f"Requested library {library_key} not found.")
- elif isinstance(library, LibraryRootV1):
- if filter_children:
- # Apply simple filtering based on CAPA problem types:
- source_blocks.extend(_problem_type_filter(store, library, dest_block.capa_type))
- else:
- source_blocks.extend(library.children)
- with store.bulk_operations(dest_block.scope_ids.usage_id.context_key):
- try:
- dest_block.source_library_version = str(library.location.library_key.version_guid)
- store.update_item(dest_block, user_id)
- dest_block.children = store.copy_from_template(
- source_blocks, dest_block.location, user_id, head_validation=True
- )
- # ^-- copy_from_template updates the children in the DB
- # but we must also set .children here to avoid overwriting the DB again
- except Exception as exception: # pylint: disable=broad-except
- TASK_LOGGER.exception('Error importing children for %s', dest_block.scope_ids.usage_id, exc_info=True)
- if task.status.state != UserTaskStatus.FAILED:
- task.status.fail({'raw_error_msg': str(exception)})
- raise
- elif isinstance(library, library_api.ContentLibraryMetadata):
- # TODO: add filtering by capa_type when V2 library will support different problem types
+ return
+ filter_children = (dest_block.capa_type != ANY_CAPA_TYPE_VALUE)
+ if filter_children:
+ # Apply simple filtering based on CAPA problem types:
+ source_blocks.extend(_problem_type_filter(store, library, dest_block.capa_type))
+ else:
+ source_blocks.extend(library.children)
+ with store.bulk_operations(dest_block.scope_ids.usage_id.context_key):
try:
- source_block_ids = [
- str(library_api.LibraryXBlockMetadata.from_component(library_key, component).usage_key)
- for component in library_api.get_library_components(library_key)
- ]
- _import_from_learning_core(user_id, store, dest_block, source_block_ids)
- dest_block.source_library_version = str(library.version)
+ dest_block.source_library_version = str(library.location.library_key.version_guid)
store.update_item(dest_block, user_id)
+ dest_block.children = store.copy_from_template(
+ source_blocks, dest_block.location, user_id, head_validation=True
+ )
+ # ^-- copy_from_template updates the children in the DB
+ # but we must also set .children here to avoid overwriting the DB again
except Exception as exception: # pylint: disable=broad-except
TASK_LOGGER.exception('Error importing children for %s', dest_block.scope_ids.usage_id, exc_info=True)
if task.status.state != UserTaskStatus.FAILED:
@@ -354,8 +215,8 @@ def _sync_children(
def _copy_overrides(
store: MixedModuleStore,
user_id: int,
- source_block: LibraryContentBlock,
- dest_block: LibraryContentBlock
+ source_block: LegacyLibraryContentBlock,
+ dest_block: LegacyLibraryContentBlock
) -> None:
"""
Copy any overrides the user has made on children of `source` over to the children of `dest_block`, recursively.
diff --git a/openedx/core/djangoapps/content_libraries/tests/base.py b/openedx/core/djangoapps/content_libraries/tests/base.py
index 987133255f58..a046206e302f 100644
--- a/openedx/core/djangoapps/content_libraries/tests/base.py
+++ b/openedx/core/djangoapps/content_libraries/tests/base.py
@@ -36,6 +36,7 @@
URL_LIB_LTI_LAUNCH = URL_LIB_LTI_PREFIX + 'launch/'
URL_BLOCK_RENDER_VIEW = '/api/xblock/v2/xblocks/{block_key}/view/{view_name}/'
+URL_BLOCK_EMBED_VIEW = '/xblocks/v2/{block_key}/embed/{view_name}/' # Returns HTML not JSON so its URL is different
URL_BLOCK_GET_HANDLER_URL = '/api/xblock/v2/xblocks/{block_key}/handler_url/{handler_name}/'
URL_BLOCK_METADATA_URL = '/api/xblock/v2/xblocks/{block_key}/'
URL_BLOCK_FIELDS_URL = '/api/xblock/v2/xblocks/{block_key}/fields/'
@@ -300,6 +301,24 @@ def _render_block_view(self, block_key, view_name, expect_response=200):
url = URL_BLOCK_RENDER_VIEW.format(block_key=block_key, view_name=view_name)
return self._api('get', url, None, expect_response)
+ def _embed_block(
+ self,
+ block_key,
+ *,
+ view_name="student_view",
+ version: str | int | None = None,
+ expect_response=200,
+ ) -> str:
+ """
+ Get an HTML response that displays the given XBlock. Returns HTML.
+ """
+ url = URL_BLOCK_EMBED_VIEW.format(block_key=block_key, view_name=view_name)
+ if version is not None:
+ url += f"?version={version}"
+ response = self.client.get(url)
+ assert response.status_code == expect_response, 'Unexpected response code {}:'.format(response.status_code)
+ return response.content.decode()
+
def _get_block_handler_url(self, block_key, handler_name):
"""
Get the URL to call a specific XBlock's handler.
@@ -308,3 +327,12 @@ def _get_block_handler_url(self, block_key, handler_name):
"""
url = URL_BLOCK_GET_HANDLER_URL.format(block_key=block_key, handler_name=handler_name)
return self._api('get', url, None, expect_response=200)["handler_url"]
+
+ def _get_library_block_fields(self, block_key, expect_response=200):
+ """ Get the fields of a specific block in the library. This API is only used by the MFE editors. """
+ result = self._api('get', URL_BLOCK_FIELDS_URL.format(block_key=block_key), None, expect_response)
+ return result
+
+ def _set_library_block_fields(self, block_key, new_fields, expect_response=200):
+ """ Set the fields of a specific block in the library. This API is only used by the MFE editors. """
+ return self._api('post', URL_BLOCK_FIELDS_URL.format(block_key=block_key), new_fields, expect_response)
diff --git a/openedx/core/djangoapps/content_libraries/tests/fields_test_block.py b/openedx/core/djangoapps/content_libraries/tests/fields_test_block.py
new file mode 100644
index 000000000000..0db5ac28d7a8
--- /dev/null
+++ b/openedx/core/djangoapps/content_libraries/tests/fields_test_block.py
@@ -0,0 +1,60 @@
+"""
+Block for testing variously scoped XBlock fields.
+"""
+import json
+
+from webob import Response
+from web_fragments.fragment import Fragment
+from xblock.core import XBlock, Scope
+from xblock import fields
+
+
+class FieldsTestBlock(XBlock):
+ """
+ Block for testing variously scoped XBlock fields and XBlock handlers.
+
+ This has only authored fields. See also UserStateTestBlock which has user fields.
+ """
+ BLOCK_TYPE = "fields-test"
+ has_score = False
+
+ display_name = fields.String(scope=Scope.settings, name='User State Test Block')
+ setting_field = fields.String(scope=Scope.settings, name='A setting')
+ content_field = fields.String(scope=Scope.content, name='A setting')
+
+ @XBlock.json_handler
+ def update_fields(self, data, suffix=None): # pylint: disable=unused-argument
+ """
+ Update the authored fields of this block
+ """
+ self.display_name = data["display_name"]
+ self.setting_field = data["setting_field"]
+ self.content_field = data["content_field"]
+ return {}
+
+ @XBlock.handler
+ def get_fields(self, request, suffix=None): # pylint: disable=unused-argument
+ """
+ Get the various fields of this XBlock.
+ """
+ return Response(
+ json.dumps({
+ "display_name": self.display_name,
+ "setting_field": self.setting_field,
+ "content_field": self.content_field,
+ }),
+ content_type='application/json',
+ charset='UTF-8',
+ )
+
+ def student_view(self, _context):
+ """
+ Return the student view.
+ """
+ fragment = Fragment()
+ fragment.add_content(f'
\n')
+ return fragment
diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py
index 8041c508dc31..c526e7b1a1f3 100644
--- a/openedx/core/djangoapps/content_libraries/tests/test_api.py
+++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py
@@ -308,6 +308,13 @@ def setUp(self):
description="Description for Collection 2",
created_by=self.user.id,
)
+ self.col3 = api.create_library_collection(
+ self.lib2.library_key,
+ collection_key="COL3",
+ title="Collection 3",
+ description="Description for Collection 3",
+ created_by=self.user.id,
+ )
# Create some library blocks in lib1
self.lib1_problem_block = self._add_block_to_library(
@@ -316,6 +323,10 @@ def setUp(self):
self.lib1_html_block = self._add_block_to_library(
self.lib1.library_key, "html", "html1",
)
+ # Create some library blocks in lib2
+ self.lib2_problem_block = self._add_block_to_library(
+ self.lib2.library_key, "problem", "problem2",
+ )
def test_create_library_collection(self):
event_receiver = mock.Mock()
@@ -498,3 +509,55 @@ def test_update_collection_components_from_wrong_library(self):
],
)
assert self.lib1_problem_block["id"] in str(exc.exception)
+
+ def test_set_library_component_collections(self):
+ event_receiver = mock.Mock()
+ CONTENT_OBJECT_ASSOCIATIONS_CHANGED.connect(event_receiver)
+ collection_update_event_receiver = mock.Mock()
+ LIBRARY_COLLECTION_UPDATED.connect(collection_update_event_receiver)
+ assert not list(self.col2.entities.all())
+ component = api.get_component_from_usage_key(UsageKey.from_string(self.lib2_problem_block["id"]))
+
+ api.set_library_component_collections(
+ self.lib2.library_key,
+ component,
+ collection_keys=[self.col2.key, self.col3.key],
+ )
+
+ assert len(authoring_api.get_collection(self.lib2.learning_package_id, self.col2.key).entities.all()) == 1
+ assert len(authoring_api.get_collection(self.lib2.learning_package_id, self.col3.key).entities.all()) == 1
+ self.assertDictContainsSubset(
+ {
+ "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED,
+ "sender": None,
+ "content_object": ContentObjectChangedData(
+ object_id=self.lib2_problem_block["id"],
+ changes=["collections"],
+ ),
+ },
+ event_receiver.call_args_list[0].kwargs,
+ )
+ self.assertDictContainsSubset(
+ {
+ "signal": LIBRARY_COLLECTION_UPDATED,
+ "sender": None,
+ "library_collection": LibraryCollectionData(
+ self.lib2.library_key,
+ collection_key=self.col2.key,
+ background=True,
+ ),
+ },
+ collection_update_event_receiver.call_args_list[0].kwargs,
+ )
+ self.assertDictContainsSubset(
+ {
+ "signal": LIBRARY_COLLECTION_UPDATED,
+ "sender": None,
+ "library_collection": LibraryCollectionData(
+ self.lib2.library_key,
+ collection_key=self.col3.key,
+ background=True,
+ ),
+ },
+ collection_update_event_receiver.call_args_list[1].kwargs,
+ )
diff --git a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py
index d995a2c79683..03b32e08ad36 100644
--- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py
+++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py
@@ -502,6 +502,30 @@ def test_library_blocks_type_constrained(self, slug, library_type, block_type, e
# Add a 'problem' XBlock to the library:
self._add_block_to_library(lib_id, block_type, 'test-block', expect_response=expect_response)
+ def test_library_not_found(self):
+ """Test that requests fail with 404 when the library does not exist"""
+ valid_not_found_key = 'lb:valid:key:video:1'
+ response = self.client.get(URL_BLOCK_METADATA_URL.format(block_key=valid_not_found_key))
+ self.assertEqual(response.status_code, 404)
+ self.assertEqual(response.json(), {
+ 'detail': "Content Library 'lib:valid:key' does not exist",
+ })
+
+ def test_block_not_found(self):
+ """Test that requests fail with 404 when the library exists but the XBlock does not"""
+ lib = self._create_library(
+ slug="test_lib_block_event_delete",
+ title="Event Test Library",
+ description="Testing event in library"
+ )
+ library_key = LibraryLocatorV2.from_string(lib['id'])
+ non_existent_block_key = LibraryUsageLocatorV2(lib_key=library_key, block_type='video', usage_id='123')
+ response = self.client.get(URL_BLOCK_METADATA_URL.format(block_key=non_existent_block_key))
+ self.assertEqual(response.status_code, 404)
+ self.assertEqual(response.json(), {
+ 'detail': f"The component '{non_existent_block_key}' does not exist.",
+ })
+
# Test that permissions are enforced for content libraries
def test_library_permissions(self): # pylint: disable=too-many-statements
@@ -635,22 +659,28 @@ def test_library_permissions(self): # pylint: disable=too-many-statements
# A random user cannot read OLX nor assets (this library has allow_public_read False):
with self.as_user(random_user):
self._get_library_block_olx(block3_key, expect_response=403)
+ self._get_library_block_fields(block3_key, expect_response=403)
self._get_library_block_assets(block3_key, expect_response=403)
- self._get_library_block_asset(block3_key, file_name="whatever.png", expect_response=403)
+ self._get_library_block_asset(block3_key, file_name="static/whatever.png", expect_response=403)
+ # Nor can they preview the block:
+ self._render_block_view(block3_key, view_name="student_view", expect_response=403)
# But if we grant allow_public_read, then they can:
with self.as_user(admin):
self._update_library(lib_id, allow_public_read=True)
- # self._set_library_block_asset(block3_key, "whatever.png", b"data")
+ self._set_library_block_asset(block3_key, "static/whatever.png", b"data")
with self.as_user(random_user):
self._get_library_block_olx(block3_key)
+ self._render_block_view(block3_key, view_name="student_view")
+ f = self._get_library_block_fields(block3_key)
# self._get_library_block_assets(block3_key)
# self._get_library_block_asset(block3_key, file_name="whatever.png")
- # Users without authoring permission cannot edit nor delete XBlocks (this library has allow_public_read False):
+ # Users without authoring permission cannot edit nor delete XBlocks:
for user in [reader, random_user]:
with self.as_user(user):
self._set_library_block_olx(block3_key, "", expect_response=403)
- # self._set_library_block_asset(block3_key, "test.txt", b"data", expect_response=403)
+ self._set_library_block_fields(block3_key, {"data": "", "metadata": {}}, expect_response=403)
+ self._set_library_block_asset(block3_key, "static/test.txt", b"data", expect_response=403)
self._delete_library_block(block3_key, expect_response=403)
self._commit_library_changes(lib_id, expect_response=403)
self._revert_library_changes(lib_id, expect_response=403)
@@ -659,9 +689,10 @@ def test_library_permissions(self): # pylint: disable=too-many-statements
with self.as_user(author_group_member):
olx = self._get_library_block_olx(block3_key)
self._set_library_block_olx(block3_key, olx)
- # self._get_library_block_assets(block3_key)
- # self._set_library_block_asset(block3_key, "test.txt", b"data")
- # self._get_library_block_asset(block3_key, file_name="test.txt")
+ self._set_library_block_fields(block3_key, {"data": olx, "metadata": {}})
+ self._get_library_block_assets(block3_key)
+ self._set_library_block_asset(block3_key, "static/test.txt", b"data")
+ self._get_library_block_asset(block3_key, file_name="static/test.txt")
self._delete_library_block(block3_key)
self._commit_library_changes(lib_id)
self._revert_library_changes(lib_id) # This is a no-op after the commit, but should still have 200 response
@@ -884,7 +915,6 @@ def test_library_block_olx_update_event(self):
event_receiver.call_args.kwargs
)
- @skip("We still need to re-implement static asset handling.")
def test_library_block_add_asset_update_event(self):
"""
Check that LIBRARY_BLOCK_CREATED event is sent when a static asset is
@@ -903,7 +933,7 @@ def test_library_block_add_asset_update_event(self):
block = self._add_block_to_library(lib_id, "unit", "u1")
block_id = block["id"]
- self._set_library_block_asset(block_id, "test.txt", b"data")
+ self._set_library_block_asset(block_id, "static/test.txt", b"data")
usage_key = LibraryUsageLocatorV2(
lib_key=library_key,
@@ -924,7 +954,6 @@ def test_library_block_add_asset_update_event(self):
event_receiver.call_args.kwargs
)
- @skip("We still need to re-implement static asset handling.")
def test_library_block_del_asset_update_event(self):
"""
Check that LIBRARY_BLOCK_CREATED event is sent when a static asset is
@@ -943,9 +972,9 @@ def test_library_block_del_asset_update_event(self):
block = self._add_block_to_library(lib_id, "unit", "u1")
block_id = block["id"]
- self._set_library_block_asset(block_id, "test.txt", b"data")
+ self._set_library_block_asset(block_id, "static/test.txt", b"data")
- self._delete_library_block_asset(block_id, 'text.txt')
+ self._delete_library_block_asset(block_id, 'static/text.txt')
usage_key = LibraryUsageLocatorV2(
lib_key=library_key,
@@ -1087,12 +1116,3 @@ def test_xblock_handler_invalid_key(self):
secure_token='random',
)))
self.assertEqual(response.status_code, 404)
-
- def test_not_found_fails_correctly(self):
- """Test fails with 404 when xblock key is valid but not found."""
- valid_not_found_key = 'lb:valid:key:video:1'
- response = self.client.get(URL_BLOCK_METADATA_URL.format(block_key=valid_not_found_key))
- self.assertEqual(response.status_code, 404)
- self.assertEqual(response.json(), {
- 'detail': f"XBlock {valid_not_found_key} does not exist, or you don't have permission to view it.",
- })
diff --git a/openedx/core/djangoapps/content_libraries/tests/test_embed_block.py b/openedx/core/djangoapps/content_libraries/tests/test_embed_block.py
new file mode 100644
index 000000000000..712117e3d245
--- /dev/null
+++ b/openedx/core/djangoapps/content_libraries/tests/test_embed_block.py
@@ -0,0 +1,184 @@
+"""
+Tests for the XBlock v2 runtime's "embed" view, using Content Libraries
+
+This view is used in the MFE to preview XBlocks that are in the library.
+"""
+import re
+
+import ddt
+from django.core.exceptions import ValidationError
+from django.test.utils import override_settings
+from openedx_events.tests.utils import OpenEdxEventsTestMixin
+import pytest
+from xblock.core import XBlock
+
+from openedx.core.djangoapps.content_libraries.tests.base import (
+ ContentLibrariesRestApiTest
+)
+from openedx.core.djangolib.testing.utils import skip_unless_cms
+from .fields_test_block import FieldsTestBlock
+
+
+@skip_unless_cms
+@ddt.ddt
+@override_settings(CORS_ORIGIN_WHITELIST=[]) # For some reason, this setting isn't defined in our test environment?
+class LibrariesEmbedViewTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMixin):
+ """
+ Tests for embed_view and interacting with draft/published/past versions of
+ Learning-Core-based XBlocks (in Content Libraries).
+
+ These tests use the REST API, which in turn relies on the Python API.
+ Some tests may use the python API directly if necessary to provide
+ coverage of any code paths not accessible via the REST API.
+
+ In general, these tests should
+ (1) Use public APIs only - don't directly create data using other methods,
+ which results in a less realistic test and ties the test suite too
+ closely to specific implementation details.
+ (Exception: users can be provisioned using a user factory)
+ (2) Assert that fields are present in responses, but don't assert that the
+ entire response has some specific shape. That way, things like adding
+ new fields to an API response, which are backwards compatible, won't
+ break any tests, but backwards-incompatible API changes will.
+
+ WARNING: every test should have a unique library slug, because even though
+ the django/mysql database gets reset for each test case, the lookup between
+ library slug and bundle UUID does not because it's assumed to be immutable
+ and cached forever.
+ """
+
+ @XBlock.register_temp_plugin(FieldsTestBlock, FieldsTestBlock.BLOCK_TYPE)
+ def test_embed_vew_versions(self):
+ """
+ Test that the embed_view renders a block and can render different versions of it.
+ """
+ # Create a library:
+ lib = self._create_library(slug="test-eb-1", title="Test Library", description="")
+ lib_id = lib["id"]
+ # Create an XBlock. This will be the empty version 1:
+ create_response = self._add_block_to_library(lib_id, FieldsTestBlock.BLOCK_TYPE, "block1")
+ block_id = create_response["id"]
+ # Create version 2 of the block by setting its OLX:
+ olx_response = self._set_library_block_olx(block_id, """
+
+ """)
+ assert olx_response["version_num"] == 2
+ # Create version 3 of the block by setting its OLX again:
+ olx_response = self._set_library_block_olx(block_id, """
+
+ """)
+ assert olx_response["version_num"] == 3
+ # Publish the library:
+ self._commit_library_changes(lib_id)
+
+ # Create the draft (version 4) of the block:
+ olx_response = self._set_library_block_olx(block_id, """
+
+ """)
+
+ # Now render the "embed block" view. This test only runs in CMS so it should default to the draft:
+ html = self._embed_block(block_id)
+
+ def check_fields(display_name, setting_value, content_value):
+ assert f'
{display_name}
' in html
+ assert f'
SF: {setting_value}
' in html
+ assert f'
CF: {content_value}
' in html
+ handler_url = re.search(r'
handler URL: ([^<]+)
', html).group(1)
+ assert handler_url.startswith('http')
+ handler_result = self.client.get(handler_url).json()
+ assert handler_result == {
+ "display_name": display_name,
+ "setting_field": setting_value,
+ "content_field": content_value,
+ }
+ check_fields('Field Test Block (Draft, v4)', 'Draft setting value 4.', 'Draft content value 4.')
+
+ # But if we request the published version, we get that:
+ html = self._embed_block(block_id, version="published")
+ check_fields('Field Test Block (Published, v3)', 'Published setting value 3.', 'Published content value 3.')
+
+ # And if we request a specific version, we get that:
+ html = self._embed_block(block_id, version=3)
+ check_fields('Field Test Block (Published, v3)', 'Published setting value 3.', 'Published content value 3.')
+
+ # And if we request a specific version, we get that:
+ html = self._embed_block(block_id, version=2)
+ check_fields('Field Test Block (Old, v2)', 'Old setting value 2.', 'Old content value 2.')
+
+ html = self._embed_block(block_id, version=4)
+ check_fields('Field Test Block (Draft, v4)', 'Draft setting value 4.', 'Draft content value 4.')
+
+ @XBlock.register_temp_plugin(FieldsTestBlock, FieldsTestBlock.BLOCK_TYPE)
+ def test_handlers_modifying_published_data(self):
+ """
+ Test that if we requested any version other than "draft", the handlers should not allow _writing_ to authored
+ field data (because you'd be overwriting the latest draft version with changes based on an old version).
+
+ We may decide to relax this restriction in the future. Not sure how important it is.
+
+ Writing to student state is OK.
+ """
+ # Create a library:
+ lib = self._create_library(slug="test-eb-2", title="Test Library", description="")
+ lib_id = lib["id"]
+ # Create an XBlock. This will be the empty version 1:
+ create_response = self._add_block_to_library(lib_id, FieldsTestBlock.BLOCK_TYPE, "block1")
+ block_id = create_response["id"]
+
+ # Now render the "embed block" view. This test only runs in CMS so it should default to the draft:
+ html = self._embed_block(block_id)
+
+ def call_update_handler(**kwargs):
+ handler_url = re.search(r'
' in html
+
+ # Call the update handler to change the fields on the draft:
+ call_update_handler(display_name="DN-01", setting_field="SV-01", content_field="CV-01")
+
+ # Render the block again and check that the handler was able to update the fields:
+ html = self._embed_block(block_id)
+ check_fields(display_name="DN-01", setting_field="SV-01", content_field="CV-01")
+
+ # Publish the library:
+ self._commit_library_changes(lib_id)
+
+ # Now try changing the authored fields of the published version using a handler:
+ html = self._embed_block(block_id, version="published")
+ expected_msg = "Do not make changes to a component starting from the published or past versions."
+ with pytest.raises(ValidationError, match=expected_msg) as err:
+ call_update_handler(display_name="DN-X", setting_field="SV-X", content_field="CV-X")
+
+ # Now try changing the authored fields of a specific past version using a handler:
+ html = self._embed_block(block_id, version=2)
+ with pytest.raises(ValidationError, match=expected_msg) as err:
+ call_update_handler(display_name="DN-X", setting_field="SV-X", content_field="CV-X")
+
+ # Make sure the fields were not updated:
+ html = self._embed_block(block_id)
+ check_fields(display_name="DN-01", setting_field="SV-01", content_field="CV-01")
+
+ # TODO: test that any static assets referenced in the student_view html are loaded as the correct version, and not
+ # always loaded as "latest draft".
+
+ # TODO: if we are ever able to run these tests in the LMS, test that the LMS only allows accessing the published
+ # version.
diff --git a/openedx/core/djangoapps/content_libraries/tests/test_static_assets.py b/openedx/core/djangoapps/content_libraries/tests/test_static_assets.py
index 92ff4c1767d0..a5f69f94b174 100644
--- a/openedx/core/djangoapps/content_libraries/tests/test_static_assets.py
+++ b/openedx/core/djangoapps/content_libraries/tests/test_static_assets.py
@@ -1,11 +1,16 @@
"""
Tests for static asset files in Learning-Core-based Content Libraries
"""
-from unittest import skip
+from uuid import UUID
+from opaque_keys.edx.keys import UsageKey
+
+from common.djangoapps.student.tests.factories import UserFactory
from openedx.core.djangoapps.content_libraries.tests.base import (
ContentLibrariesRestApiTest,
)
+from openedx.core.djangoapps.xblock.api import get_component_from_usage_key
+from openedx.core.djangolib.testing.utils import skip_unless_cms
# Binary data representing an SVG image file
SVG_DATA = """
