diff --git a/cms/djangoapps/contentstore/migrations/0016_mariadb_uuid_conversion.py b/cms/djangoapps/contentstore/migrations/0016_mariadb_uuid_conversion.py new file mode 100644 index 000000000000..acba91f68370 --- /dev/null +++ b/cms/djangoapps/contentstore/migrations/0016_mariadb_uuid_conversion.py @@ -0,0 +1,74 @@ +# Generated migration for MariaDB UUID field conversion (Django 5.2) +""" +Migration to convert UUIDField from char(32) to uuid type for MariaDB compatibility. + +This migration is necessary because Django 5 changed the behavior of UUIDField for MariaDB +databases from using CharField(32) to using a proper UUID type. This change isn't managed +automatically, so we need to generate migrations to safely convert the columns. + +This migration only executes for MariaDB databases and is a no-op for other backends. + +See: https://www.albertyw.com/note/django-5-mariadb-uuidfield +""" + +from django.db import migrations + + +def apply_mariadb_migration(apps, schema_editor): + """Apply the migration only for MariaDB databases.""" + connection = schema_editor.connection + + # Check if this is a MariaDB database + if connection.vendor != 'mysql': + return + + # Additional check for MariaDB specifically (vs MySQL) + with connection.cursor() as cursor: + cursor.execute("SELECT VERSION()") + version = cursor.fetchone()[0] + if 'mariadb' not in version.lower(): + return + + # Apply the field changes for MariaDB + with connection.cursor() as cursor: + cursor.execute( + "ALTER TABLE oel_publishing_learningpackage " + "MODIFY uuid uuid NOT NULL" + ) + + +def reverse_mariadb_migration(apps, schema_editor): + """Reverse the migration only for MariaDB databases.""" + connection = schema_editor.connection + + # Check if this is a MariaDB database + if connection.vendor != 'mysql': + return + + # Additional check for MariaDB specifically (vs MySQL) + with connection.cursor() as cursor: + cursor.execute("SELECT VERSION()") + version = cursor.fetchone()[0] + if 'mariadb' not in version.lower(): + return + + # Reverse the field changes for MariaDB + with connection.cursor() as cursor: + cursor.execute( + "ALTER TABLE oel_publishing_learningpackage " + "MODIFY uuid char(32) NOT NULL" + ) + + +class Migration(migrations.Migration): + + dependencies = [ + ('contentstore', '0015_switch_to_openedx_content'), + ] + + operations = [ + migrations.RunPython( + code=apply_mariadb_migration, + reverse_code=reverse_mariadb_migration, + ), + ] diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/course_details.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/course_details.py index fbcf2aca72f4..eec4dc11f38a 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/course_details.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/course_details.py @@ -53,6 +53,7 @@ class CourseDetailsSerializer(serializers.Serializer): pre_requisite_courses = serializers.ListField(child=CourseKeyField()) run = serializers.CharField() self_paced = serializers.BooleanField() + has_changes = serializers.BooleanField() short_description = serializers.CharField(allow_blank=True) start_date = serializers.DateTimeField() subtitle = serializers.CharField(allow_blank=True) diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/course_waffle_flags.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/course_waffle_flags.py index 2d0785459069..5e6be8046b7b 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/course_waffle_flags.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/course_waffle_flags.py @@ -5,6 +5,7 @@ from rest_framework import serializers from cms.djangoapps.contentstore import toggles +from openedx.core import toggles as core_toggles class CourseWaffleFlagsSerializer(serializers.Serializer): @@ -31,6 +32,7 @@ class CourseWaffleFlagsSerializer(serializers.Serializer): use_react_markdown_editor = serializers.SerializerMethodField() use_video_gallery_flow = serializers.SerializerMethodField() enable_course_optimizer_check_prev_run_links = serializers.SerializerMethodField() + enable_authz_course_authoring = serializers.SerializerMethodField() def get_course_key(self): """ @@ -201,3 +203,10 @@ def get_enable_course_optimizer_check_prev_run_links(self, obj): """ course_key = self.get_course_key() return toggles.enable_course_optimizer_check_prev_run_links(course_key) + + def get_enable_authz_course_authoring(self, obj): + """ + Method to get the authz.enable_course_authoring waffle flag + """ + course_key = self.get_course_key() + return core_toggles.enable_authz_course_authoring(course_key) diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py index eb4f333e170a..5df4804027cb 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py @@ -125,7 +125,7 @@ class UpstreamLinkSerializer(serializers.Serializer): error_message = serializers.CharField(allow_null=True) ready_to_sync = serializers.BooleanField() downstream_customized = serializers.ListField(child=serializers.CharField(), allow_empty=True) - has_top_level_parent = serializers.BooleanField() + top_level_parent_key = serializers.CharField(allow_null=True) ready_to_sync_children = UpstreamChildrenInfoSerializer(many=True, required=False) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_waffle_flags.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_waffle_flags.py index f45cc48810d6..fd7b5882cf97 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_waffle_flags.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_waffle_flags.py @@ -38,6 +38,7 @@ class CourseWaffleFlagsViewTest(CourseTestCase): "use_react_markdown_editor": False, "use_video_gallery_flow": False, "enable_course_optimizer_check_prev_run_links": False, + "enable_authz_course_authoring": False, } def setUp(self): diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py index a7cf3a452627..cd1e1a99d074 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py @@ -323,7 +323,7 @@ def test_children_content(self): "version_declined": None, "error_message": None, "ready_to_sync": True, - "has_top_level_parent": False, + "top_level_parent_key": None, "downstream_customized": [], }, "user_partition_info": expected_user_partition_info, diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py index f32d25a6a5b0..4a3b13f9c4ab 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py @@ -50,7 +50,7 @@ def _get_upstream_link_good_and_syncable(downstream): version_declined=downstream.upstream_version_declined, error_message=None, downstream_customized=[], - has_top_level_parent=False, + top_level_parent_key=None, upstream_name=downstream.upstream_display_name, ) diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index ac9cc3d831ad..983471e1ebef 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -35,7 +35,7 @@ from olxcleaner.reporting import report_error_summary, report_errors from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey -from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocator, BlockUsageLocator +from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocator from openedx_events.content_authoring.data import CourseData from openedx_events.content_authoring.signals import COURSE_RERUN_COMPLETED from organizations.api import add_organization_course, ensure_organization @@ -1641,11 +1641,7 @@ def handle_create_xblock_upstream_link(usage_key): return if xblock.top_level_downstream_parent_key is not None: block_key = BlockKey.from_string(xblock.top_level_downstream_parent_key) - top_level_parent_usage_key = BlockUsageLocator( - xblock.course_id, - block_key.type, - block_key.id, - ) + top_level_parent_usage_key = block_key.to_usage_key(xblock.course_id) try: ContainerLink.get_by_downstream_usage_key(top_level_parent_usage_key) except ContainerLink.DoesNotExist: diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index b83158e35c76..6f7d10a9d858 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -19,6 +19,7 @@ from xblock.runtime import KvsFieldData from openedx.core.djangoapps.video_config.services import VideoConfigService +from openedx.core.djangoapps.discussions.services import DiscussionConfigService from xmodule.contentstore.django import contentstore from xmodule.exceptions import NotFoundError as XModuleNotFoundError from xmodule.modulestore.django import XBlockI18nService, modulestore @@ -228,6 +229,7 @@ def _prepare_runtime_for_preview(request, block): "cache": CacheService(cache), 'replace_urls': ReplaceURLService, 'video_config': VideoConfigService(), + 'discussion_config_service': DiscussionConfigService(), } block.runtime.get_block_for_descriptor = partial(_load_preview_block, request) diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index 5a5c380fcf58..f7c96d588a2e 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -1202,6 +1202,9 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements "edited_on": get_default_time_display(xblock.subtree_edited_on) if xblock.subtree_edited_on else None, + "edited_on_raw": str(xblock.subtree_edited_on) + if xblock.subtree_edited_on + else None, "published": published, "published_on": published_on, "studio_url": xblock_studio_url(xblock, parent_xblock), @@ -1331,7 +1334,7 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements # Disable adding or removing children component if xblock is imported from library xblock_actions["childAddable"] = False # Enable unlinking only for top level imported components - xblock_actions["unlinkable"] = not upstream_info["has_top_level_parent"] + xblock_actions["unlinkable"] = not upstream_info["top_level_parent_key"] if is_xblock_unit: # if xblock is a Unit we add the discussion_enabled option diff --git a/cms/lib/xblock/upstream_sync.py b/cms/lib/xblock/upstream_sync.py index 2b96e54c5ec9..fd3bb8a6733d 100644 --- a/cms/lib/xblock/upstream_sync.py +++ b/cms/lib/xblock/upstream_sync.py @@ -86,7 +86,7 @@ class UpstreamLink: version_declined: int | None # Latest version which the user has declined to sync with, if any. error_message: str | None # If link is valid, None. Otherwise, a localized, human-friendly error message. downstream_customized: list[str] | None # List of fields modified in downstream - has_top_level_parent: bool # True if this Upstream link has a top-level parent + top_level_parent_key: str | None # key of top-level parent if Upstream link has a one. @property def is_upstream_deleted(self) -> bool: @@ -153,7 +153,7 @@ def ready_to_sync(self) -> bool: from xmodule.modulestore.django import modulestore # If this component/container has top-level parent, so we need to sync the parent - if self.has_top_level_parent: + if self.top_level_parent_key: return False if isinstance(self.upstream_key, LibraryUsageLocatorV2): @@ -222,6 +222,10 @@ def try_get_for_block(cls, downstream: XBlock, log_error: bool = True) -> t.Self downstream.usage_key, downstream.upstream, ) + if top_level_parent_key := getattr(downstream, "top_level_downstream_parent_key", None): + top_level_parent_key = str( + BlockKey.from_string(top_level_parent_key).to_usage_key(downstream.usage_key.context_key) + ) return cls( upstream_ref=getattr(downstream, "upstream", None), upstream_name=getattr(downstream, "upstream_display_name", None), @@ -232,7 +236,7 @@ def try_get_for_block(cls, downstream: XBlock, log_error: bool = True) -> t.Self version_declined=None, error_message=str(exc), downstream_customized=getattr(downstream, "downstream_customized", []), - has_top_level_parent=getattr(downstream, "top_level_downstream_parent_key", None) is not None, + top_level_parent_key=top_level_parent_key, ) @classmethod @@ -306,6 +310,10 @@ def get_for_block(cls, downstream: XBlock) -> t.Self: ) ) + if top_level_parent_key := getattr(downstream, "top_level_downstream_parent_key", None): + top_level_parent_key = str( + BlockKey.from_string(top_level_parent_key).to_usage_key(downstream.usage_key.context_key) + ) result = cls( upstream_ref=downstream.upstream, upstream_key=upstream_key, @@ -316,7 +324,7 @@ def get_for_block(cls, downstream: XBlock) -> t.Self: version_declined=downstream.upstream_version_declined, error_message=None, downstream_customized=getattr(downstream, "downstream_customized", []), - has_top_level_parent=downstream.top_level_downstream_parent_key is not None, + top_level_parent_key=top_level_parent_key, ) return result diff --git a/cms/static/sass/course-unit-mfe-iframe-bundle.scss b/cms/static/sass/course-unit-mfe-iframe-bundle.scss index ac3a6b2cf9e7..42ce2948f0d9 100644 --- a/cms/static/sass/course-unit-mfe-iframe-bundle.scss +++ b/cms/static/sass/course-unit-mfe-iframe-bundle.scss @@ -1174,3 +1174,11 @@ select { @extend %button-primary-outline; } } + +.tooltip { + background: $primary-base; + white-space: normal; + max-width: 200px; + line-height: 1.5; + text-align: center; +} diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index ae786f9ca1b6..8191faa2744e 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -1,7 +1,7 @@ <%page expression_filter="h"/> <%! from django.utils.translation import gettext as _ -from openedx.core.djangolib.markup import Text +from openedx.core.djangolib.markup import HTML, Text from cms.djangoapps.contentstore.helpers import xblock_studio_url from cms.djangoapps.contentstore.utils import is_visible_to_specific_partition_groups, get_editor_page_base_url, determine_label from lms.lib.utils import is_unit @@ -25,7 +25,7 @@ block_is_unit = is_unit(xblock) upstream_info = UpstreamLink.try_get_for_block(xblock, log_error=False) -can_unlink = upstream_info.upstream_ref and not upstream_info.has_top_level_parent +can_unlink = upstream_info.upstream_ref and not upstream_info.top_level_parent_key %> <%namespace name='static' file='static_content.html'/> @@ -111,7 +111,7 @@ % else: % if upstream_info.upstream_ref: % if upstream_info.error_message: -
+
${_("The referenced library or library object is not available.")} + ${_("The referenced library or library object is not available")}
% elif upstream_info.ready_to_sync: - % elif len(upstream_info.downstream_customized) > 0: -
+
${_("This library reference has course overrides applied.")} + ${_("This library reference has course overrides applied")}
% else: -
+
{name}').format(name=upstream_info.upstream_name))}"> - ${_("This item is linked to a library item.")} + ${_("This item is linked to a library item")}
% endif % endif diff --git a/common/static/js/src/tooltip_manager.js b/common/static/js/src/tooltip_manager.js index 71db826a705d..bc30a6641154 100644 --- a/common/static/js/src/tooltip_manager.js +++ b/common/static/js/src/tooltip_manager.js @@ -69,8 +69,10 @@ pageX = typeof pageX !== 'undefined' ? pageX : element.offset().left + element.width() / 2; pageY = typeof pageY !== 'undefined' ? pageY : element.offset().top + element.height() / 2; var tooltipText = $(element).attr('data-tooltip'); + // Tooltip content comes from data-tooltip attributes which are server-rendered + // with proper escaping using Text() and HTML() from openedx.core.djangolib.markup this.tooltip - .text(tooltipText) + .html(tooltipText) // xss-lint: disable=javascript-jquery-html .css(this.getCoords(pageX, pageY)); }, diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index b6e4145e2ecf..767d4033a73f 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -43,6 +43,7 @@ from lms.djangoapps.teams.services import TeamsService from openedx.core.djangoapps.video_config.services import VideoConfigService +from openedx.core.djangoapps.discussions.services import DiscussionConfigService from openedx.core.lib.xblock_services.call_to_action import CallToActionService from xmodule.contentstore.django import contentstore from xmodule.exceptions import NotFoundError as XModuleNotFoundError @@ -637,6 +638,7 @@ def inner_get_block(block: XBlock) -> XBlock | None: 'publish': EventPublishingService(user, course_id, track_function), 'enrollments': EnrollmentsService(), 'video_config': VideoConfigService(), + 'discussion_config_service': DiscussionConfigService(), } runtime.get_block_for_descriptor = inner_get_block diff --git a/lms/djangoapps/courseware/tests/test_discussion_xblock.py b/lms/djangoapps/courseware/tests/test_discussion_xblock.py index 16c40e8ff5d0..7ea82a0cf600 100644 --- a/lms/djangoapps/courseware/tests/test_discussion_xblock.py +++ b/lms/djangoapps/courseware/tests/test_discussion_xblock.py @@ -28,6 +28,7 @@ from lms.djangoapps.courseware.block_render import get_block_for_descriptor from lms.djangoapps.courseware.tests.helpers import XModuleRenderingTestBase from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, Provider +from openedx.core.djangoapps.discussions.services import DiscussionConfigService from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory @@ -193,6 +194,14 @@ def test_student_perms_are_correct(self, permissions): 'can_create_subcomment': permission_dict['create_sub_comment'], } + self.add_patcher( + patch.multiple( + DiscussionConfigService, + is_discussion_visible=mock.Mock(return_value=True), + is_discussion_enabled=mock.Mock(return_value=True) + ) + ) + self.block.has_permission = lambda perm: permission_dict[perm] with mock.patch('xmodule.discussion_block.render_to_string', return_value='') as mock_render: self.block.student_view() @@ -223,13 +232,10 @@ def test_has_permission(self): Test for has_permission method. """ permission_canary = object() - with mock.patch( - 'xmodule.discussion_block.has_permission', - return_value=permission_canary, - ) as has_perm: - actual_permission = self.block.has_permission("test_permission") + self.block.has_permission = mock.Mock(return_value=permission_canary) + actual_permission = self.block.has_permission("test_permission") assert actual_permission == permission_canary - has_perm.assert_called_once_with(self.django_user_canary, 'test_permission', self.course_id) + self.block.has_permission.assert_called_once_with("test_permission") def test_studio_view(self): """Test for studio view.""" @@ -252,6 +258,14 @@ def test_student_perms_are_correct(self, permissions): 'create_sub_comment': permissions[2] } + self.add_patcher( + patch.multiple( + DiscussionConfigService, + is_discussion_visible=mock.Mock(return_value=True), + is_discussion_enabled=mock.Mock(return_value=True) + ) + ) + self.block.has_permission = lambda perm: permission_dict[perm] fragment = self.block.student_view() read_only = 'false' if permissions[0] else 'true' @@ -296,7 +310,7 @@ def get_root(self, block): block = block.get_parent() return block - @override_settings(FEATURES=dict(settings.FEATURES, ENABLE_DISCUSSION_SERVICE='True')) + @override_settings(ENABLE_DISCUSSION_SERVICE=True) def test_html_with_user(self): """ Test rendered DiscussionXBlock permissions. @@ -317,7 +331,7 @@ def test_html_with_user(self): assert 'data-user-create-comment="false"' in html assert 'data-user-create-subcomment="false"' in html - @override_settings(FEATURES=dict(settings.FEATURES, ENABLE_DISCUSSION_SERVICE='True')) + @override_settings(ENABLE_DISCUSSION_SERVICE=True) def test_discussion_render_successfully_with_orphan_parent(self): """ Test that discussion xblock render successfully @@ -421,7 +435,7 @@ class TestXBlockQueryLoad(SharedModuleStoreTestCase): Test the number of queries executed when rendering the XBlock. """ - @override_settings(FEATURES=dict(settings.FEATURES, ENABLE_DISCUSSION_SERVICE='True')) + @override_settings(ENABLE_DISCUSSION_SERVICE=True) def test_permissions_query_load(self): """ Tests that the permissions queries are cached when rendering numerous discussion XBlocks. diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 47b55e1090c0..583d5ee64b66 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -71,8 +71,6 @@ from lms.djangoapps.courseware.tests.helpers import MasqueradeMixin, get_expiration_banner_text from lms.djangoapps.courseware.testutils import RenderXBlockTestMixin from lms.djangoapps.courseware.toggles import ( - COURSEWARE_MICROFRONTEND_ALWAYS_OPEN_AUXILIARY_SIDEBAR, - COURSEWARE_MICROFRONTEND_ENABLE_NAVIGATION_SIDEBAR, COURSEWARE_MICROFRONTEND_SEARCH_ENABLED, COURSEWARE_OPTIMIZED_RENDER_XBLOCK, ) @@ -3251,13 +3249,10 @@ def setUp(self): self.client = APIClient() self.apiUrl = reverse('courseware_navigation_sidebar_toggles_view', kwargs={'course_id': str(self.course.id)}) - @override_waffle_flag(COURSEWARE_MICROFRONTEND_ENABLE_NAVIGATION_SIDEBAR, active=True) - @override_waffle_flag(COURSEWARE_MICROFRONTEND_ALWAYS_OPEN_AUXILIARY_SIDEBAR, active=False) @override_waffle_switch(ENABLE_COMPLETION_TRACKING_SWITCH, active=False) - def test_courseware_mfe_navigation_sidebar_enabled_aux_disabled_completion_track_disabled(self): + def test_courseware_mfe_navigation_sidebar_completion_track_disabled(self): """ - Getter to check if it is allowed to show the Courseware navigation sidebar to a user - and auxiliary sidebar doesn't open. + Getter to check if completion tracking is disabled. """ response = self.client.get(self.apiUrl, content_type='application/json') body = json.loads(response.content.decode('utf-8')) @@ -3266,19 +3261,14 @@ def test_courseware_mfe_navigation_sidebar_enabled_aux_disabled_completion_track self.assertEqual( body, { - "enable_navigation_sidebar": True, - "always_open_auxiliary_sidebar": False, "enable_completion_tracking": False, }, ) - @override_waffle_flag(COURSEWARE_MICROFRONTEND_ENABLE_NAVIGATION_SIDEBAR, active=True) - @override_waffle_flag(COURSEWARE_MICROFRONTEND_ALWAYS_OPEN_AUXILIARY_SIDEBAR, active=False) @override_waffle_switch(ENABLE_COMPLETION_TRACKING_SWITCH, active=True) - def test_courseware_mfe_navigation_sidebar_enabled_aux_disabled_completion_track_enabled(self): + def test_courseware_mfe_navigation_sidebar_completion_track_enabled(self): """ - Getter to check if it is allowed to show the Courseware navigation sidebar to a user - and auxiliary sidebar doesn't open. + Getter to check if completion tracking is enabled. """ response = self.client.get(self.apiUrl, content_type='application/json') body = json.loads(response.content.decode('utf-8')) @@ -3287,132 +3277,6 @@ def test_courseware_mfe_navigation_sidebar_enabled_aux_disabled_completion_track self.assertEqual( body, { - "enable_navigation_sidebar": True, - "always_open_auxiliary_sidebar": False, - "enable_completion_tracking": True, - }, - ) - - @override_waffle_flag(COURSEWARE_MICROFRONTEND_ENABLE_NAVIGATION_SIDEBAR, active=True) - @override_waffle_flag(COURSEWARE_MICROFRONTEND_ALWAYS_OPEN_AUXILIARY_SIDEBAR, active=True) - @override_waffle_switch(ENABLE_COMPLETION_TRACKING_SWITCH, active=False) - def test_courseware_mfe_navigation_sidebar_enabled_aux_enabled_completion_track_disabled(self): - """ - Getter to check if it is allowed to show the Courseware navigation sidebar to a user - and auxiliary sidebar should always open. - """ - response = self.client.get(self.apiUrl, content_type='application/json') - body = json.loads(response.content.decode('utf-8')) - - self.assertEqual(response.status_code, 200) - self.assertEqual( - body, - { - "enable_navigation_sidebar": True, - "always_open_auxiliary_sidebar": True, - "enable_completion_tracking": False, - }, - ) - - @override_waffle_flag(COURSEWARE_MICROFRONTEND_ENABLE_NAVIGATION_SIDEBAR, active=True) - @override_waffle_flag(COURSEWARE_MICROFRONTEND_ALWAYS_OPEN_AUXILIARY_SIDEBAR, active=True) - @override_waffle_switch(ENABLE_COMPLETION_TRACKING_SWITCH, active=True) - def test_courseware_mfe_navigation_sidebar_enabled_aux_enabled_completion_track_enabled(self): - """ - Getter to check if it is allowed to show the Courseware navigation sidebar to a user - and auxiliary sidebar should always open. - """ - response = self.client.get(self.apiUrl, content_type='application/json') - body = json.loads(response.content.decode('utf-8')) - - self.assertEqual(response.status_code, 200) - self.assertEqual( - body, - { - "enable_navigation_sidebar": True, - "always_open_auxiliary_sidebar": True, - "enable_completion_tracking": True, - }, - ) - - @override_waffle_flag(COURSEWARE_MICROFRONTEND_ENABLE_NAVIGATION_SIDEBAR, active=False) - @override_waffle_flag(COURSEWARE_MICROFRONTEND_ALWAYS_OPEN_AUXILIARY_SIDEBAR, active=True) - @override_waffle_switch(ENABLE_COMPLETION_TRACKING_SWITCH, active=False) - def test_courseware_mfe_navigation_sidebar_disabled_aux_enabled_completion_track_disabled(self): - """ - Getter to check if the Courseware navigation sidebar shouldn't be shown to a user - and auxiliary sidebar should always open. - """ - response = self.client.get(self.apiUrl, content_type='application/json') - body = json.loads(response.content.decode('utf-8')) - - self.assertEqual(response.status_code, 200) - self.assertEqual( - body, - { - "enable_navigation_sidebar": False, - "always_open_auxiliary_sidebar": True, - "enable_completion_tracking": False, - }, - ) - - @override_waffle_flag(COURSEWARE_MICROFRONTEND_ENABLE_NAVIGATION_SIDEBAR, active=False) - @override_waffle_flag(COURSEWARE_MICROFRONTEND_ALWAYS_OPEN_AUXILIARY_SIDEBAR, active=True) - @override_waffle_switch(ENABLE_COMPLETION_TRACKING_SWITCH, active=True) - def test_courseware_mfe_navigation_sidebar_disabled_aux_enabled_completion_track_enabled(self): - """ - Getter to check if the Courseware navigation sidebar shouldn't be shown to a user - and auxiliary sidebar should always open. - """ - response = self.client.get(self.apiUrl, content_type='application/json') - body = json.loads(response.content.decode('utf-8')) - - self.assertEqual(response.status_code, 200) - self.assertEqual( - body, - { - "enable_navigation_sidebar": False, - "always_open_auxiliary_sidebar": True, - "enable_completion_tracking": True, - }, - ) - - @override_waffle_flag(COURSEWARE_MICROFRONTEND_ENABLE_NAVIGATION_SIDEBAR, active=False) - @override_waffle_flag(COURSEWARE_MICROFRONTEND_ALWAYS_OPEN_AUXILIARY_SIDEBAR, active=False) - @override_waffle_switch(ENABLE_COMPLETION_TRACKING_SWITCH, active=False) - def test_courseware_mfe_navigation_sidebar_toggles_disabled_completion_track_disabled(self): - """ - Getter to check if neither navigation sidebar nor auxiliary sidebar is shown. - """ - response = self.client.get(self.apiUrl, content_type='application/json') - body = json.loads(response.content.decode('utf-8')) - - self.assertEqual(response.status_code, 200) - self.assertEqual( - body, - { - "enable_navigation_sidebar": False, - "always_open_auxiliary_sidebar": False, - "enable_completion_tracking": False, - }, - ) - - @override_waffle_flag(COURSEWARE_MICROFRONTEND_ENABLE_NAVIGATION_SIDEBAR, active=False) - @override_waffle_flag(COURSEWARE_MICROFRONTEND_ALWAYS_OPEN_AUXILIARY_SIDEBAR, active=False) - @override_waffle_switch(ENABLE_COMPLETION_TRACKING_SWITCH, active=True) - def test_courseware_mfe_navigation_sidebar_toggles_disabled_completion_track_enabled(self): - """ - Getter to check if neither navigation sidebar nor auxiliary sidebar is shown. - """ - response = self.client.get(self.apiUrl, content_type='application/json') - body = json.loads(response.content.decode('utf-8')) - - self.assertEqual(response.status_code, 200) - self.assertEqual( - body, - { - "enable_navigation_sidebar": False, - "always_open_auxiliary_sidebar": False, "enable_completion_tracking": True, }, ) diff --git a/lms/djangoapps/courseware/toggles.py b/lms/djangoapps/courseware/toggles.py index 9b295a948c16..a14924519757 100644 --- a/lms/djangoapps/courseware/toggles.py +++ b/lms/djangoapps/courseware/toggles.py @@ -83,33 +83,6 @@ f'{WAFFLE_FLAG_NAMESPACE}.disable_navigation_sidebar_blocks_caching', __name__ ) -# .. toggle_name: courseware.enable_navigation_sidebar -# .. toggle_implementation: WaffleFlag -# .. toggle_default: False -# .. toggle_description: Enable navigation sidebar on Learning MFE -# .. toggle_use_cases: opt_out, open_edx -# .. toggle_creation_date: 2024-03-07 -# .. toggle_target_removal_date: None -# .. toggle_tickets: FC-0056 -COURSEWARE_MICROFRONTEND_ENABLE_NAVIGATION_SIDEBAR = CourseWaffleFlag( - f'{WAFFLE_FLAG_NAMESPACE}.enable_navigation_sidebar', __name__ -) - -# .. toggle_name: courseware.always_open_auxiliary_sidebar -# .. toggle_implementation: WaffleFlag -# .. toggle_default: True -# .. toggle_description: Waffle flag that determines whether the auxiliary sidebar, -# such as discussion or notification, should automatically expand -# on each course unit page within the Learning MFE, without preserving -# the previous state of the sidebar. -# .. toggle_use_cases: temporary -# .. toggle_creation_date: 2024-04-28 -# .. toggle_target_removal_date: 2024-07-28 -# .. toggle_tickets: FC-0056 -COURSEWARE_MICROFRONTEND_ALWAYS_OPEN_AUXILIARY_SIDEBAR = CourseWaffleFlag( - f'{WAFFLE_FLAG_NAMESPACE}.always_open_auxiliary_sidebar', __name__ -) - # .. toggle_name: courseware.mfe_progress_milestones_streak_discount_enabled # .. toggle_implementation: CourseWaffleFlag # .. toggle_default: False diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 7aaf97ae837f..c37034016f39 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -101,8 +101,6 @@ from lms.djangoapps.courseware.toggles import ( course_is_invitation_only, courseware_mfe_search_is_enabled, - COURSEWARE_MICROFRONTEND_ENABLE_NAVIGATION_SIDEBAR, - COURSEWARE_MICROFRONTEND_ALWAYS_OPEN_AUXILIARY_SIDEBAR, ) from completion.waffle import ENABLE_COMPLETION_TRACKING_SWITCH from lms.djangoapps.courseware.user_state_client import DjangoXBlockUserStateClient @@ -2398,8 +2396,6 @@ def courseware_mfe_navigation_sidebar_toggles(request, course_id=None): return JsonResponse({"error": "Invalid course_id"}) return JsonResponse({ - "enable_navigation_sidebar": COURSEWARE_MICROFRONTEND_ENABLE_NAVIGATION_SIDEBAR.is_enabled(course_key), - "always_open_auxiliary_sidebar": COURSEWARE_MICROFRONTEND_ALWAYS_OPEN_AUXILIARY_SIDEBAR.is_enabled(course_key), # Add completion tracking status for the sidebar use while a global place for switches is put in place "enable_completion_tracking": ENABLE_COMPLETION_TRACKING_SWITCH.is_enabled() }) diff --git a/lms/djangoapps/discussion/django_comment_client/base/views.py b/lms/djangoapps/discussion/django_comment_client/base/views.py index 14ce9c4b575a..e40ee4ef58bb 100644 --- a/lms/djangoapps/discussion/django_comment_client/base/views.py +++ b/lms/djangoapps/discussion/django_comment_client/base/views.py @@ -25,6 +25,7 @@ import lms.djangoapps.discussion.django_comment_client.settings as cc_settings import openedx.core.djangoapps.django_comment_common.comment_client as cc +from openedx.core.djangoapps.django_comment_common.models import has_permission from common.djangoapps.student.roles import GlobalStaff from common.djangoapps.track import contexts from common.djangoapps.util.file import store_uploaded_file @@ -33,8 +34,7 @@ from lms.djangoapps.courseware.exceptions import CourseAccessRedirect from lms.djangoapps.discussion.django_comment_client.permissions import ( check_permissions_by_view, - get_team, - has_permission + get_team ) from lms.djangoapps.discussion.django_comment_client.utils import ( JsonError, diff --git a/lms/djangoapps/discussion/django_comment_client/permissions.py b/lms/djangoapps/discussion/django_comment_client/permissions.py index 2eeee32fe722..4801a461c608 100644 --- a/lms/djangoapps/discussion/django_comment_client/permissions.py +++ b/lms/djangoapps/discussion/django_comment_client/permissions.py @@ -12,26 +12,11 @@ from openedx.core.djangoapps.django_comment_common.comment_client import Thread from openedx.core.djangoapps.django_comment_common.models import ( CourseDiscussionSettings, - all_permissions_for_user_in_course + has_permission ) from openedx.core.lib.cache_utils import request_cached -def has_permission(user, permission, course_id=None): # lint-amnesty, pylint: disable=missing-function-docstring - assert isinstance(course_id, (type(None), CourseKey)) - request_cache_dict = DEFAULT_REQUEST_CACHE.data - cache_key = "django_comment_client.permissions.has_permission.all_permissions.{}.{}".format( - user.id, course_id - ) - if cache_key in request_cache_dict: - all_permissions = request_cache_dict[cache_key] - else: - all_permissions = all_permissions_for_user_in_course(user, course_id) - request_cache_dict[cache_key] = all_permissions - - return permission in all_permissions - - CONDITIONS = ['is_open', 'is_author', 'is_question_author', 'is_team_member_if_applicable'] diff --git a/lms/djangoapps/discussion/django_comment_client/utils.py b/lms/djangoapps/discussion/django_comment_client/utils.py index e26b748270e3..a0bb6b769183 100644 --- a/lms/djangoapps/discussion/django_comment_client/utils.py +++ b/lms/djangoapps/discussion/django_comment_client/utils.py @@ -24,10 +24,10 @@ from lms.djangoapps.discussion.django_comment_client.constants import TYPE_ENTRY, TYPE_SUBCATEGORY from lms.djangoapps.discussion.django_comment_client.permissions import ( check_permissions_by_view, - get_team, - has_permission + get_team ) from lms.djangoapps.discussion.django_comment_client.settings import MAX_COMMENT_DEPTH +from openedx.core.djangoapps.django_comment_common.models import has_permission from openedx.core.djangoapps.course_groups.cohorts import get_cohort_id from openedx.core.djangoapps.discussions.utils import ( get_accessible_discussion_xblocks, diff --git a/lms/djangoapps/discussion/templates/discussion/discussion_profile_page.html b/lms/djangoapps/discussion/templates/discussion/discussion_profile_page.html index f88c33440ce7..90f03999d539 100644 --- a/lms/djangoapps/discussion/templates/discussion/discussion_profile_page.html +++ b/lms/djangoapps/discussion/templates/discussion/discussion_profile_page.html @@ -11,7 +11,7 @@ from django.template.defaultfilters import escapejs from django.urls import reverse -from lms.djangoapps.discussion.django_comment_client.permissions import has_permission +from openedx.core.djangoapps.django_comment_common.models import has_permission from openedx.core.djangolib.js_utils import dump_js_escaped_json, js_escaped_string %> diff --git a/lms/djangoapps/discussion/views.py b/lms/djangoapps/discussion/views.py index d6f61d209433..bca6cb7768de 100644 --- a/lms/djangoapps/discussion/views.py +++ b/lms/djangoapps/discussion/views.py @@ -28,6 +28,7 @@ import lms.djangoapps.discussion.django_comment_client.utils as utils import openedx.core.djangoapps.django_comment_common.comment_client as cc +from openedx.core.djangoapps.django_comment_common.models import has_permission from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, GlobalStaff from common.djangoapps.util.json_request import JsonResponse, expect_json @@ -37,7 +38,6 @@ from lms.djangoapps.discussion.config.settings import is_forum_daily_digest_enabled from lms.djangoapps.discussion.django_comment_client.base.views import track_thread_viewed_event from lms.djangoapps.discussion.django_comment_client.constants import TYPE_ENTRY -from lms.djangoapps.discussion.django_comment_client.permissions import has_permission from lms.djangoapps.discussion.django_comment_client.utils import ( add_courseware_context, course_discussion_division_enabled, diff --git a/mypy.ini b/mypy.ini index 982e500b3a93..6cb44c9300a2 100644 --- a/mypy.ini +++ b/mypy.ini @@ -15,6 +15,7 @@ files = # openedx/core/djangoapps/content/search, openedx/core/djangoapps/content_staging, openedx/core/djangoapps/content_libraries, + openedx/core/djangoapps/discussions/services.py, openedx/core/djangoapps/programs/rest_api, openedx/core/djangoapps/xblock, openedx/core/lib/derived.py, diff --git a/openedx/core/djangoapps/discussions/services.py b/openedx/core/djangoapps/discussions/services.py new file mode 100644 index 000000000000..6ccee6099bac --- /dev/null +++ b/openedx/core/djangoapps/discussions/services.py @@ -0,0 +1,38 @@ +""" +Discussion Configuration Service for XBlock runtime. + +This service provides discussion-related configuration and feature flags +that are specific to the edx-platform implementation +for the extracted discussion block in xblocks-contrib repository. +""" + +from django.conf import settings +from django.contrib.auth.models import User # pylint: disable=imported-auth-user +from opaque_keys.edx.keys import CourseKey +from openedx.core.djangoapps.django_comment_common.models import has_permission +from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, Provider + + +class DiscussionConfigService: + """ + Service for providing discussion-related configuration and feature flags. + """ + + def has_permission(self, user: User, permission: str, course_id: CourseKey | None = None) -> bool: + """ + Return whether the user has the given discussion permission for a given course. + """ + return has_permission(user, permission, course_id) + + def is_discussion_visible(self, course_key: CourseKey) -> bool: + """ + Discussion Xblock does not support new OPEN_EDX provider + """ + provider = DiscussionsConfiguration.get(course_key) + return provider.provider_type == Provider.LEGACY + + def is_discussion_enabled(self) -> bool: + """ + Return True if discussions are enabled; else False + """ + return settings.ENABLE_DISCUSSION_SERVICE diff --git a/openedx/core/djangoapps/django_comment_common/models.py b/openedx/core/djangoapps/django_comment_common/models.py index bd7b8fe66e67..51863c42d472 100644 --- a/openedx/core/djangoapps/django_comment_common/models.py +++ b/openedx/core/djangoapps/django_comment_common/models.py @@ -14,6 +14,8 @@ from django.utils.translation import gettext_noop from jsonfield.fields import JSONField from opaque_keys.edx.django.models import CourseKeyField +from edx_django_utils.cache import DEFAULT_REQUEST_CACHE +from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.xmodule_django.models import NoneToEmptyManager from openedx.core.lib.cache_utils import request_cached @@ -193,6 +195,37 @@ def all_permissions_for_user_in_course(user, course_id): return permission_names +def has_permission(user, permission, course_id=None): + """ + This function resolves all discussion-related permissions for the given + user and course, caches them for the duration of the request, and verifies + whether the requested permission is present. + + Args: + user (User): Django user whose permissions are being checked. + permission (str): Discussion permission identifier + (e.g., "create_comment", "create_thread"). + course_id (CourseKey): Course context in which to evaluate + the permission + + Returns: + bool: True if the user has the specified permission in the given + course context; False otherwise. + """ + assert isinstance(course_id, (type(None), CourseKey)) + request_cache_dict = DEFAULT_REQUEST_CACHE.data + cache_key = "django_comment_client.permissions.has_permission.all_permissions.{}.{}".format( + user.id, course_id + ) + if cache_key in request_cache_dict: + all_permissions = request_cache_dict[cache_key] + else: + all_permissions = all_permissions_for_user_in_course(user, course_id) + request_cache_dict[cache_key] = all_permissions + + return permission in all_permissions + + class ForumsConfig(ConfigurationModel): """ Config for the connection to the cs_comments_service forums backend. diff --git a/openedx/core/djangoapps/models/course_details.py b/openedx/core/djangoapps/models/course_details.py index c90081f30a03..95473adf21d4 100644 --- a/openedx/core/djangoapps/models/course_details.py +++ b/openedx/core/djangoapps/models/course_details.py @@ -77,6 +77,7 @@ def __init__(self, org, course_id, run): self.self_paced = None self.learning_info = [] self.instructor_info = [] + self.has_changes = None @classmethod def fetch_about_attribute(cls, course_key, attribute): @@ -127,6 +128,7 @@ def populate(cls, block): course_details.video_thumbnail_image_asset_path = course_image_url(block, 'video_thumbnail_image') course_details.language = block.language course_details.self_paced = block.self_paced + course_details.has_changes = modulestore().has_changes(block) course_details.learning_info = block.learning_info course_details.instructor_info = block.instructor_info course_details.title = block.display_name diff --git a/openedx/core/djangoapps/xblock/runtime/runtime.py b/openedx/core/djangoapps/xblock/runtime/runtime.py index 2ae4a431bfbe..041450d8a341 100644 --- a/openedx/core/djangoapps/xblock/runtime/runtime.py +++ b/openedx/core/djangoapps/xblock/runtime/runtime.py @@ -347,6 +347,9 @@ def service(self, block: XBlock, service_name: str): # Import here to avoid circular dependency from openedx.core.djangoapps.video_config.services import VideoConfigService return VideoConfigService() + elif service_name == 'discussion_config_service': + from openedx.core.djangoapps.discussions.services import DiscussionConfigService + return DiscussionConfigService() # Otherwise, fall back to the base implementation which loads services # defined in the constructor: diff --git a/openedx/core/toggles.py b/openedx/core/toggles.py index b4f704dca2e1..fd966a95cf80 100644 --- a/openedx/core/toggles.py +++ b/openedx/core/toggles.py @@ -3,6 +3,8 @@ for them. Generally speaking, they should be added to the most appropriate app or repo. """ from edx_toggles.toggles import SettingToggle +from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag + # .. toggle_name: ENTRANCE_EXAMS # .. toggle_implementation: SettingToggle @@ -15,3 +17,21 @@ ENTRANCE_EXAMS = SettingToggle( "ENTRANCE_EXAMS", default=False, module_name=__name__ ) + +# .. toggle_name: authz.enable_course_authoring +# .. toggle_implementation: CourseWaffleFlag +# .. toggle_default: False +# .. toggle_description: This toggle will enable the new openedx-authz authorization engine for course authoring. +# .. toggle_warning: Enabling this toggle will trigger a data migration to move role assignations between the legacy and the openedx-authz system. +# .. toggle_use_cases: temporary +# .. toggle_creation_date: 2026-02-05 +# .. toggle_target_removal_date: 2027-06-09 +# .. toggle_tickets: https://github.com/openedx/openedx-platform/issues/37927 +AUTHZ_COURSE_AUTHORING_FLAG = CourseWaffleFlag('authz.enable_course_authoring', __name__) + + +def enable_authz_course_authoring(course_key): + """ + Returns a boolean if the AuthZ for course authoring feature is enabled for the given course. + """ + return AUTHZ_COURSE_AUTHORING_FLAG.is_enabled(course_key) diff --git a/openedx/features/course_bookmarks/templates/course_bookmarks/course-bookmarks.html b/openedx/features/course_bookmarks/templates/course_bookmarks/course-bookmarks.html index a7038b3bdae6..bda15a7431af 100644 --- a/openedx/features/course_bookmarks/templates/course_bookmarks/course-bookmarks.html +++ b/openedx/features/course_bookmarks/templates/course_bookmarks/course-bookmarks.html @@ -15,7 +15,7 @@ from django.utils.translation import gettext as _ from django.template.defaultfilters import escapejs -from lms.djangoapps.discussion.django_comment_client.permissions import has_permission +from openedx.core.djangoapps.django_comment_common.models import has_permission from openedx.core.djangolib.js_utils import dump_js_escaped_json, js_escaped_string from openedx.core.djangolib.markup import HTML from openedx.features.course_experience import course_home_page_title diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 34b93fc2878d..6a04252e4203 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -708,8 +708,6 @@ lazy==1.6 # lti-consumer-xblock # ora2 # xblock -loremipsum==1.0.5 - # via ora2 lti-consumer-xblock==9.14.3 # via -r requirements/edx/kernel.in lxml[html-clean]==5.3.2 @@ -840,7 +838,7 @@ openedx-learning==0.31.0 # -r requirements/edx/kernel.in optimizely-sdk==5.4.0 # via -r requirements/edx/bundled.in -ora2==6.17.1 +ora2==6.17.2 # via -r requirements/edx/bundled.in packaging==25.0 # via diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 9e4be4c4e967..566f9df1bc2c 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1190,11 +1190,6 @@ libsass==0.10.0 # via # -c requirements/constraints.txt # -r requirements/edx/assets.txt -loremipsum==1.0.5 - # via - # -r requirements/edx/doc.txt - # -r requirements/edx/testing.txt - # ora2 lti-consumer-xblock==9.14.3 # via # -r requirements/edx/doc.txt @@ -1413,7 +1408,7 @@ optimizely-sdk==5.4.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -ora2==6.17.1 +ora2==6.17.2 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 6b8cbf58f849..a7f3fb2f094b 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -865,10 +865,6 @@ lazy==1.6 # lti-consumer-xblock # ora2 # xblock -loremipsum==1.0.5 - # via - # -r requirements/edx/base.txt - # ora2 lti-consumer-xblock==9.14.3 # via -r requirements/edx/base.txt lxml[html-clean]==5.3.2 @@ -1019,7 +1015,7 @@ openedx-learning==0.31.0 # -r requirements/edx/base.txt optimizely-sdk==5.4.0 # via -r requirements/edx/base.txt -ora2==6.17.1 +ora2==6.17.2 # via -r requirements/edx/base.txt packaging==25.0 # via diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index db4497c94a16..307d80fffa3c 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -907,10 +907,6 @@ lazy==1.6 # lti-consumer-xblock # ora2 # xblock -loremipsum==1.0.5 - # via - # -r requirements/edx/base.txt - # ora2 lti-consumer-xblock==9.14.3 # via -r requirements/edx/base.txt lxml[html-clean]==5.3.2 @@ -1069,7 +1065,7 @@ openedx-learning==0.31.0 # -r requirements/edx/base.txt optimizely-sdk==5.4.0 # via -r requirements/edx/base.txt -ora2==6.17.1 +ora2==6.17.2 # via -r requirements/edx/base.txt packaging==25.0 # via diff --git a/xmodule/discussion_block.py b/xmodule/discussion_block.py index aaea2de7bb2a..97bb724930c5 100644 --- a/xmodule/discussion_block.py +++ b/xmodule/discussion_block.py @@ -17,8 +17,6 @@ from xblock.utils.studio_editable import StudioEditableXBlockMixin from xblocks_contrib.discussion import DiscussionXBlock as _ExtractedDiscussionXBlock -from lms.djangoapps.discussion.django_comment_client.permissions import has_permission -from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, Provider from openedx.core.djangolib.markup import HTML, Text from openedx.core.lib.xblock_utils import get_css_dependencies, get_js_dependencies from xmodule.xml_block import XmlMixin @@ -37,6 +35,7 @@ def _(text): @XBlock.needs('user') # pylint: disable=abstract-method @XBlock.needs('i18n') @XBlock.needs('mako') +@XBlock.wants('discussion_config_service') class _BuiltInDiscussionXBlock(XBlock, StudioEditableXBlockMixin, XmlMixin): # lint-amnesty, pylint: disable=abstract-method """ @@ -76,6 +75,13 @@ class _BuiltInDiscussionXBlock(XBlock, StudioEditableXBlockMixin, has_author_view = True # Tells Studio to use author_view + @property + def discussion_config_service(self): + """ + Returns discussion configuration service. + """ + return self.runtime.service(self, 'discussion_config_service') + @property def course_key(self): return getattr(self.scope_ids.usage_id, 'course_key', None) @@ -85,8 +91,18 @@ def is_visible(self): """ Discussion Xblock does not support new OPEN_EDX provider """ - provider = DiscussionsConfiguration.get(self.course_key) - return provider.provider_type == Provider.LEGACY + if self.discussion_config_service: + return self.discussion_config_service.is_discussion_visible(self.course_key) + return False + + @property + def is_discussion_enabled(self): + """ + Returns True if discussions are enabled; else False + """ + if self.discussion_config_service: + return self.discussion_config_service.is_discussion_enabled() + return False @property def django_user(self): @@ -159,15 +175,14 @@ def has_permission(self, permission): :param str permission: Permission :rtype: bool """ - return has_permission(self.django_user, permission, self.course_key) + if self.discussion_config_service: + return self.discussion_config_service.has_permission(self.django_user, permission, self.course_key) + return False def student_view(self, context=None): """ Renders student view for LMS. """ - # to prevent a circular import issue - import lms.djangoapps.discussion.django_comment_client.utils as utils - fragment = Fragment() if not self.is_visible: @@ -193,7 +208,7 @@ def student_view(self, context=None): url='{}?{}'.format(reverse('register_user'), qs), ), ) - if utils.is_discussion_enabled(self.course_key): + if self.is_discussion_enabled: context = { 'discussion_id': self.discussion_id, 'display_name': self.display_name if self.display_name else _("Discussion"), @@ -282,8 +297,17 @@ def _apply_metadata_and_policy(cls, block, node, runtime): setattr(block, field_name, value) -DiscussionXBlock = ( - _ExtractedDiscussionXBlock if settings.USE_EXTRACTED_DISCUSSION_BLOCK - else _BuiltInDiscussionXBlock -) +DiscussionXBlock = None + + +def reset_class(): + """Reset class as per django settings flag""" + global DiscussionXBlock + DiscussionXBlock = ( + _ExtractedDiscussionXBlock if settings.USE_EXTRACTED_DISCUSSION_BLOCK + else _BuiltInDiscussionXBlock + ) + return DiscussionXBlock + +reset_class() DiscussionXBlock.__name__ = "DiscussionXBlock" diff --git a/xmodule/tests/__init__.py b/xmodule/tests/__init__.py index c6db0acb45a1..c6db16a8fbc6 100644 --- a/xmodule/tests/__init__.py +++ b/xmodule/tests/__init__.py @@ -33,7 +33,7 @@ from xmodule.x_module import DoNothingCache, XModuleMixin, ModuleStoreRuntime from openedx.core.djangoapps.video_config.services import VideoConfigService from openedx.core.lib.cache_utils import CacheService - +from openedx.core.djangoapps.discussions.services import DiscussionConfigService MODULE_DIR = path(__file__).dirname() # Location of common test DATA directory @@ -161,6 +161,7 @@ def get_block(block): 'field-data': DictFieldData({}), 'sandbox': SandboxService(contentstore, course_id), 'video_config': VideoConfigService(), + 'discussion_config_service': DiscussionConfigService() } descriptor_system.get_block_for_descriptor = get_block # lint-amnesty, pylint: disable=attribute-defined-outside-init @@ -217,6 +218,7 @@ def get_block(block): 'field-data': DictFieldData({}), 'sandbox': SandboxService(contentstore, course_id), 'video_config': VideoConfigService(), + 'discussion_config_service': DiscussionConfigService() } if add_overrides: diff --git a/xmodule/tests/test_util_keys.py b/xmodule/tests/test_util_keys.py index ceda27847830..6ab54716181b 100644 --- a/xmodule/tests/test_util_keys.py +++ b/xmodule/tests/test_util_keys.py @@ -1,15 +1,15 @@ """ Tests for xmodule/util/keys.py """ -import ddt -import pytest from unittest import TestCase from unittest.mock import Mock -from opaque_keys.edx.locator import BlockUsageLocator +import ddt +import pytest from opaque_keys.edx.keys import CourseKey -from xmodule.util.keys import BlockKey, derive_key +from opaque_keys.edx.locator import BlockUsageLocator +from xmodule.util.keys import BlockKey, derive_key mock_block = Mock() mock_block.id = CourseKey.from_string('course-v1:Beeper+B33P+BOOP') @@ -70,3 +70,19 @@ def test_block_key_from_string_error(self, block_key_str): @ddt.unpack def test_block_key_to_string(self, block_key, block_key_str): assert str(block_key) == block_key_str + + @ddt.data( + [BlockKey('chapter', 'some-id'), BlockUsageLocator( + mock_block.id, + 'chapter', + 'some-id' + )], + [BlockKey('section', 'one-more-id'), BlockUsageLocator( + mock_block.id, + 'section', + 'one-more-id' + )] + ) + @ddt.unpack + def test_block_key_to_usage_key(self, block_key: BlockKey, block_key_str): + assert block_key.to_usage_key(mock_block.id) == block_key_str diff --git a/xmodule/util/keys.py b/xmodule/util/keys.py index 9570079200cc..d2837e217de2 100644 --- a/xmodule/util/keys.py +++ b/xmodule/util/keys.py @@ -6,7 +6,7 @@ import hashlib from typing import NamedTuple, Self -from opaque_keys.edx.keys import UsageKey +from opaque_keys.edx.keys import CourseKey, UsageKey class BlockKey(NamedTuple): @@ -40,6 +40,12 @@ def from_string(cls, s: str) -> Self: raise ValueError(f"Invalid string format for BlockKey: {s}") return cls(parts[0], parts[1]) + def to_usage_key(self, course_key: CourseKey) -> UsageKey: + """ + Converts this BlockKey into a UsageKey. + """ + return course_key.make_usage_key(self.type, self.id) + def derive_key(source: UsageKey, dest_parent: BlockKey) -> BlockKey: """