From 0cd62bf786cb436388298f33e8e1195551d9f3a1 Mon Sep 17 00:00:00 2001 From: farhan Date: Fri, 10 Oct 2025 18:56:17 +0500 Subject: [PATCH] refactor: Introduce VideoConfig service, move video sharing methods in it --- cms/djangoapps/contentstore/views/preview.py | 4 +- lms/djangoapps/courseware/block_render.py | 2 + .../courseware/tests/test_video_mongo.py | 25 +++--- lms/djangoapps/courseware/views/views.py | 3 +- .../core/djangoapps/video_config/services.py | 64 +++++++++++++++ .../core/djangoapps/video_config/sharing.py | 81 +++++++++++++++++++ xmodule/course_block.py | 8 +- xmodule/tests/__init__.py | 6 +- xmodule/video_block/video_block.py | 72 ++--------------- 9 files changed, 182 insertions(+), 83 deletions(-) create mode 100644 openedx/core/djangoapps/video_config/services.py create mode 100644 openedx/core/djangoapps/video_config/sharing.py diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 4f8312fdb3..1c64cfb660 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -18,6 +18,7 @@ from xblock.django.request import django_to_webob_request, webob_to_django_respo from xblock.exceptions import NoSuchHandlerError from xblock.runtime import KvsFieldData +from openedx.core.djangoapps.video_config.services import VideoConfigService from xmodule.contentstore.django import contentstore from xmodule.exceptions import NotFoundError, ProcessingError from xmodule.modulestore.django import XBlockI18nService, modulestore @@ -214,7 +215,8 @@ def _prepare_runtime_for_preview(request, block): "teams_configuration": TeamsConfigurationService(), "sandbox": SandboxService(contentstore=contentstore, course_id=course_id), "cache": CacheService(cache), - 'replace_urls': ReplaceURLService + 'replace_urls': ReplaceURLService, + 'video_config': VideoConfigService(), } block.runtime.get_block_for_descriptor = partial(_load_preview_block, request) diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index 6e83c3a123..ad04bcb192 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -42,6 +42,7 @@ from xblock.reference.plugins import FSService from xblock.runtime import KvsFieldData from lms.djangoapps.teams.services import TeamsService +from openedx.core.djangoapps.video_config.services import VideoConfigService from openedx.core.lib.xblock_services.call_to_action import CallToActionService from xmodule.contentstore.django import contentstore from xmodule.exceptions import NotFoundError, ProcessingError @@ -635,6 +636,7 @@ def prepare_runtime_for_user( 'call_to_action': CallToActionService(), 'publish': EventPublishingService(user, course_id, track_function), 'enrollments': EnrollmentsService(), + 'video_config': VideoConfigService(), } runtime.get_block_for_descriptor = inner_get_block diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index 48f45267ca..b5a8646032 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -37,7 +37,7 @@ from fs.path import combine from lxml import etree from path import Path as path from xmodule.contentstore.content import StaticContent -from xmodule.course_block import ( +from openedx.core.djangoapps.video_config.sharing import ( COURSE_VIDEO_SHARING_ALL_VIDEOS, COURSE_VIDEO_SHARING_NONE, COURSE_VIDEO_SHARING_PER_VIDEO @@ -57,6 +57,7 @@ from xmodule.x_module import PUBLIC_VIEW, STUDENT_VIEW from common.djangoapps.xblock_django.constants import ATTR_KEY_REQUEST_COUNTRY_CODE from lms.djangoapps.courseware.tests.helpers import get_context_dict_from_string from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE +from openedx.core.djangoapps.video_config import sharing from openedx.core.djangoapps.video_pipeline.config.waffle import DEPRECATE_YOUTUBE from openedx.core.djangoapps.waffle_utils.models import WaffleFlagCourseOverrideModel from openedx.core.djangolib.testing.utils import CacheIsolationTestCase @@ -260,14 +261,14 @@ class TestVideoPublicAccess(BaseTestVideoXBlock): """Test public video url.""" assert self.block.public_access is True with self.mock_feature_toggle(enabled=feature_enabled): - assert self.block.is_public_sharing_enabled() == feature_enabled + assert sharing.is_public_sharing_enabled(self.block.location, self.block.public_access) == feature_enabled def test_is_public_sharing_enabled__not_public(self): self.block.public_access = False with self.mock_feature_toggle(): - assert not self.block.is_public_sharing_enabled() + assert not sharing.is_public_sharing_enabled(self.block.location, self.block.public_access) - @patch('xmodule.video_block.video_block.VideoBlock.get_course_video_sharing_override') + @patch('openedx.core.djangoapps.video_config.sharing.get_course_video_sharing_override') def test_is_public_sharing_enabled_by_course_override(self, mock_course_sharing_override): # Given a course overrides all videos to be shared @@ -276,12 +277,12 @@ class TestVideoPublicAccess(BaseTestVideoXBlock): # When I try to determine if public sharing is enabled with self.mock_feature_toggle(): - is_public_sharing_enabled = self.block.is_public_sharing_enabled() + is_public_sharing_enabled = sharing.is_public_sharing_enabled(self.block.location, self.block.public_access) # Then I will get that course value self.assertTrue(is_public_sharing_enabled) - @patch('xmodule.video_block.video_block.VideoBlock.get_course_video_sharing_override') + @patch('openedx.core.djangoapps.video_config.sharing.get_course_video_sharing_override') def test_is_public_sharing_disabled_by_course_override(self, mock_course_sharing_override): # Given a course overrides no videos to be shared mock_course_sharing_override.return_value = COURSE_VIDEO_SHARING_NONE @@ -289,13 +290,13 @@ class TestVideoPublicAccess(BaseTestVideoXBlock): # When I try to determine if public sharing is enabled with self.mock_feature_toggle(): - is_public_sharing_enabled = self.block.is_public_sharing_enabled() + is_public_sharing_enabled = sharing.is_public_sharing_enabled(self.block.location, self.block.public_access) # Then I will get that course value self.assertFalse(is_public_sharing_enabled) @ddt.data(COURSE_VIDEO_SHARING_PER_VIDEO, None) - @patch('xmodule.video_block.video_block.VideoBlock.get_course_video_sharing_override') + @patch('openedx.core.djangoapps.video_config.sharing.get_course_video_sharing_override') def test_is_public_sharing_enabled_per_video(self, mock_override_value, mock_course_sharing_override): # Given a course does not override per-video settings mock_course_sharing_override.return_value = mock_override_value @@ -303,12 +304,12 @@ class TestVideoPublicAccess(BaseTestVideoXBlock): # When I try to determine if public sharing is enabled with self.mock_feature_toggle(): - is_public_sharing_enabled = self.block.is_public_sharing_enabled() + is_public_sharing_enabled = sharing.is_public_sharing_enabled(self.block.location, self.block.public_access) # I will get the per-video value self.assertEqual(self.block.public_access, is_public_sharing_enabled) - @patch('xmodule.video_block.video_block.get_course_by_id') + @patch('openedx.core.lib.courses.get_course_by_id') def test_is_public_sharing_course_not_found(self, mock_get_course): # Given a course does not override per-video settings mock_get_course.side_effect = Http404() @@ -316,7 +317,7 @@ class TestVideoPublicAccess(BaseTestVideoXBlock): # When I try to determine if public sharing is enabled with self.mock_feature_toggle(): - is_public_sharing_enabled = self.block.is_public_sharing_enabled() + is_public_sharing_enabled = sharing.is_public_sharing_enabled(self.block.location, self.block.public_access) # I will fall-back to per-video values self.assertEqual(self.block.public_access, is_public_sharing_enabled) @@ -325,7 +326,7 @@ class TestVideoPublicAccess(BaseTestVideoXBlock): def test_context(self, is_public_sharing_enabled): with self.mock_feature_toggle(): with patch.object( - self.block, + sharing, 'is_public_sharing_enabled', return_value=is_public_sharing_enabled ): diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 2c89800d82..7aaf97ae83 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -138,6 +138,7 @@ from openedx.core.djangoapps.programs.utils import ProgramMarketingDataExtender from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.util.user_messages import PageLevelMessages from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE +from openedx.core.djangoapps.video_config.sharing import is_public_sharing_enabled from openedx.core.djangoapps.zendesk_proxy.utils import create_zendesk_ticket from openedx.core.djangolib.markup import HTML, Text from openedx.core.lib.courses import get_course_by_id @@ -1869,7 +1870,7 @@ class BasePublicVideoXBlockView(View): ) # Block must be marked as public to be viewed - if not video_block.is_public_sharing_enabled(): + if not is_public_sharing_enabled(video_block.location, video_block.public_access): raise Http404("Video not found.") return course, video_block diff --git a/openedx/core/djangoapps/video_config/services.py b/openedx/core/djangoapps/video_config/services.py new file mode 100644 index 0000000000..4ab5e5c4bd --- /dev/null +++ b/openedx/core/djangoapps/video_config/services.py @@ -0,0 +1,64 @@ +""" +Video Configuration Service for XBlock runtime. + +This service provides video-related configuration and feature flags +that are specific to the edx-platform implementation +for the extracted video block in xblocks-contrib repository. +""" + +import logging + +from opaque_keys.edx.keys import CourseKey, UsageKey + +from openedx.core.djangoapps.video_config import sharing +from organizations.api import get_course_organization + + +log = logging.getLogger(__name__) + + +class VideoConfigService: + """ + Service for providing video-related configuration and feature flags. + + This service abstracts away edx-platform specific functionality + that the Video XBlock needs, allowing the Video XBlock to be + extracted to a separate repository. + """ + + def get_public_video_url(self, usage_id: UsageKey) -> str: + """ + Returns the public video url + """ + return sharing.get_public_video_url(usage_id) + + def get_public_sharing_context(self, video_block, course_key: CourseKey) -> dict: + """ + Get the complete public sharing context for a video. + + Args: + video_block: The video XBlock instance + course_key: The course identifier + + Returns: + dict: Context dictionary with sharing information, empty if sharing is disabled + """ + context = {} + + if not sharing.is_public_sharing_enabled(video_block.location, video_block.public_access): + return context + + public_video_url = sharing.get_public_video_url(video_block.location) + context['public_sharing_enabled'] = True + context['public_video_url'] = public_video_url + + organization = get_course_organization(course_key) + + from openedx.core.djangoapps.video_config.sharing_sites import sharing_sites_info_for_video + sharing_sites_info = sharing_sites_info_for_video( + public_video_url, + organization=organization + ) + context['sharing_sites_info'] = sharing_sites_info + + return context diff --git a/openedx/core/djangoapps/video_config/sharing.py b/openedx/core/djangoapps/video_config/sharing.py new file mode 100644 index 0000000000..4cc66cdb05 --- /dev/null +++ b/openedx/core/djangoapps/video_config/sharing.py @@ -0,0 +1,81 @@ +""" +Provides utility methods for video sharing functionality. +""" + +import logging + +from django.conf import settings +from opaque_keys.edx.keys import UsageKey + +from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE +from openedx.core.lib.courses import get_course_by_id + +log = logging.getLogger(__name__) + +# Video sharing constants +COURSE_VIDEO_SHARING_PER_VIDEO = 'per-video' +COURSE_VIDEO_SHARING_ALL_VIDEOS = 'all-on' +COURSE_VIDEO_SHARING_NONE = 'all-off' + + +@staticmethod +def get_public_video_url(usage_id: UsageKey) -> str: + """ + Returns the public video url + """ + return fr'{settings.LMS_ROOT_URL}/videos/{str(usage_id)}' + + +@staticmethod +def is_public_sharing_enabled(usage_key: UsageKey, public_access: bool) -> bool: + """ + Check if public sharing is enabled for a video. + + Args: + usage_key: The usage key of the video block + public_access: Whether the video block has public access enabled + """ + if not usage_key.context_key.is_course: + return False # Only courses support this feature (not libraries) + + try: + # Video share feature must be enabled for sharing settings to take effect + feature_enabled = PUBLIC_VIDEO_SHARE.is_enabled(usage_key.context_key) + except Exception as err: # pylint: disable=broad-except + log.exception(f"Error retrieving course for course ID: {usage_key.context_key}") + return False + + if not feature_enabled: + return False + + # Check if the course specifies a general setting + course_video_sharing_option = get_course_video_sharing_override(usage_key) + + # Course can override all videos to be shared + if course_video_sharing_option == COURSE_VIDEO_SHARING_ALL_VIDEOS: + return True + + # ... or no videos to be shared + elif course_video_sharing_option == COURSE_VIDEO_SHARING_NONE: + return False + + # ... or can fall back to per-video setting + # Equivalent to COURSE_VIDEO_SHARING_PER_VIDEO or None / unset + else: + return public_access + + +@staticmethod +def get_course_video_sharing_override(usage_key: UsageKey) -> str | None: + """ + Return course video sharing options override + """ + if not usage_key.context_key.is_course: + return False # Only courses support this feature (not libraries) + + try: + course = get_course_by_id(usage_key.context_key) + return getattr(course, 'video_sharing_options', None) + except Exception as err: # pylint: disable=broad-except + log.exception(f"Error retrieving course for course ID: {usage_key.context_key}") + return None diff --git a/xmodule/course_block.py b/xmodule/course_block.py index c3f42ecb66..9d24f15306 100644 --- a/xmodule/course_block.py +++ b/xmodule/course_block.py @@ -18,6 +18,11 @@ from path import Path as path from pytz import utc from xblock.fields import Boolean, Dict, Float, Integer, List, Scope, String from openedx.core.djangoapps.video_pipeline.models import VideoUploadsEnabledByDefault +from openedx.core.djangoapps.video_config.sharing import ( + COURSE_VIDEO_SHARING_ALL_VIDEOS, + COURSE_VIDEO_SHARING_NONE, + COURSE_VIDEO_SHARING_PER_VIDEO, +) from openedx.core.lib.license import LicenseMixin from openedx.core.lib.teams_config import TeamsConfig # lint-amnesty, pylint: disable=unused-import from xmodule import course_metadata_utils @@ -55,9 +60,6 @@ COURSE_VISIBILITY_PRIVATE = 'private' COURSE_VISIBILITY_PUBLIC_OUTLINE = 'public_outline' COURSE_VISIBILITY_PUBLIC = 'public' -COURSE_VIDEO_SHARING_PER_VIDEO = 'per-video' -COURSE_VIDEO_SHARING_ALL_VIDEOS = 'all-on' -COURSE_VIDEO_SHARING_NONE = 'all-off' # .. toggle_name: FEATURES['CREATE_COURSE_WITH_DEFAULT_ENROLLMENT_START_DATE'] # .. toggle_implementation: SettingDictToggle # .. toggle_default: False diff --git a/xmodule/tests/__init__.py b/xmodule/tests/__init__.py index 819c89669e..c6db0acb45 100644 --- a/xmodule/tests/__init__.py +++ b/xmodule/tests/__init__.py @@ -31,6 +31,7 @@ from xmodule.modulestore.xml import CourseLocationManager from xmodule.tests.helpers import StubReplaceURLService, mock_render_template, StubMakoService, StubUserService from xmodule.util.sandboxing import SandboxService 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 @@ -159,6 +160,7 @@ def get_test_system( 'cache': CacheService(DoNothingCache()), 'field-data': DictFieldData({}), 'sandbox': SandboxService(contentstore, course_id), + 'video_config': VideoConfigService(), } descriptor_system.get_block_for_descriptor = get_block # lint-amnesty, pylint: disable=attribute-defined-outside-init @@ -214,6 +216,7 @@ def prepare_block_runtime( 'cache': CacheService(DoNothingCache()), 'field-data': DictFieldData({}), 'sandbox': SandboxService(contentstore, course_id), + 'video_config': VideoConfigService(), } if add_overrides: @@ -241,6 +244,7 @@ def get_test_descriptor_system(render_template=None, **kwargs): Construct a test ModuleStoreRuntime instance. """ field_data = DictFieldData({}) + video_config = VideoConfigService() descriptor_system = TestModuleStoreRuntime( load_item=Mock(name='get_test_descriptor_system.load_item'), @@ -248,7 +252,7 @@ def get_test_descriptor_system(render_template=None, **kwargs): error_tracker=Mock(name='get_test_descriptor_system.error_tracker'), render_template=render_template or mock_render_template, mixins=(InheritanceMixin, XModuleMixin), - services={'field-data': field_data}, + services={'field-data': field_data, 'video_config': video_config}, **kwargs ) descriptor_system.get_asides = lambda block: [] diff --git a/xmodule/video_block/video_block.py b/xmodule/video_block/video_block.py index 7c6c26c13b..f2c7182832 100644 --- a/xmodule/video_block/video_block.py +++ b/xmodule/video_block/video_block.py @@ -23,7 +23,6 @@ from django.conf import settings from edx_django_utils.cache import RequestCache from lxml import etree from opaque_keys.edx.locator import AssetLocator -from organizations.api import get_course_organization from web_fragments.fragment import Fragment from xblock.completable import XBlockCompletionMode from xblock.core import XBlock @@ -33,16 +32,11 @@ from xblocks_contrib.video import VideoBlock as _ExtractedVideoBlock from common.djangoapps.xblock_django.constants import ATTR_KEY_REQUEST_COUNTRY_CODE, ATTR_KEY_USER_ID from openedx.core.djangoapps.video_config.models import HLSPlaybackEnabledFlag, CourseYoutubeBlockedFlag -from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE, TRANSCRIPT_FEEDBACK +from openedx.core.djangoapps.video_config.toggles import TRANSCRIPT_FEEDBACK from openedx.core.djangoapps.video_pipeline.config.waffle import DEPRECATE_YOUTUBE from openedx.core.lib.cache_utils import request_cached -from openedx.core.lib.courses import get_course_by_id from openedx.core.lib.license import LicenseMixin from xmodule.contentstore.content import StaticContent -from xmodule.course_block import ( - COURSE_VIDEO_SHARING_ALL_VIDEOS, - COURSE_VIDEO_SHARING_NONE, -) from xmodule.editing_block import EditingMixin from xmodule.exceptions import NotFoundError from xmodule.mako_block import MakoTemplateBlockBase @@ -58,7 +52,6 @@ from xmodule.x_module import ( ) from xmodule.xml_block import XmlMixin, deserialize_field, is_pointer_tag, name_to_pathname from .bumper_utils import bumperize -from openedx.core.djangoapps.video_config.sharing_sites import sharing_sites_info_for_video from .transcripts_utils import ( Transcript, VideoTranscriptsMixin, @@ -114,7 +107,7 @@ EXPORT_IMPORT_STATIC_DIR = 'static' @XBlock.wants('settings', 'completion', 'i18n', 'request_cache') -@XBlock.needs('mako', 'user') +@XBlock.needs('mako', 'user', 'video_config') class _BuiltInVideoBlock( VideoFields, VideoTranscriptsMixin, VideoStudioViewHandlers, VideoStudentViewHandlers, EmptyDataRawMixin, XmlMixin, EditingMixin, XModuleToXBlockMixin, @@ -492,64 +485,12 @@ class _BuiltInVideoBlock( 'transcript_download_formats_list': self.fields['transcript_download_format'].values, # lint-amnesty, pylint: disable=unsubscriptable-object 'transcript_feedback_enabled': self.is_transcript_feedback_enabled(), } - if self.is_public_sharing_enabled(): - public_video_url = self.get_public_video_url() - template_context['public_sharing_enabled'] = True - template_context['public_video_url'] = public_video_url - organization = get_course_organization(self.course_id) - template_context['sharing_sites_info'] = sharing_sites_info_for_video( - public_video_url, - organization=organization - ) + video_config_service = self.runtime.service(self, 'video_config') + if video_config_service: + template_context.update(video_config_service.get_public_sharing_context(self, self.course_id)) return self.runtime.service(self, 'mako').render_lms_template('video.html', template_context) - def get_course_video_sharing_override(self): - """ - Return course video sharing options override or None - """ - if not self.context_key.is_course: - return False # Only courses support this feature at all (not libraries) - try: - course = get_course_by_id(self.context_key) - return getattr(course, 'video_sharing_options', None) - - # In case the course / modulestore does something weird - except Exception as err: # pylint: disable=broad-except - log.exception(f"Error retrieving course for course ID: {self.course_id}") - return None - - def is_public_sharing_enabled(self): - """ - Is public sharing enabled for this video? - """ - if not self.context_key.is_course: - return False # Only courses support this feature at all (not libraries) - try: - # Video share feature must be enabled for sharing settings to take effect - feature_enabled = PUBLIC_VIDEO_SHARE.is_enabled(self.context_key) - except Exception as err: # pylint: disable=broad-except - log.exception(f"Error retrieving course for course ID: {self.context_key}") - return False - if not feature_enabled: - return False - - # Check if the course specifies a general setting - course_video_sharing_option = self.get_course_video_sharing_override() - - # Course can override all videos to be shared - if course_video_sharing_option == COURSE_VIDEO_SHARING_ALL_VIDEOS: - return True - - # ... or no videos to be shared - elif course_video_sharing_option == COURSE_VIDEO_SHARING_NONE: - return False - - # ... or can fall back to per-video setting - # Equivalent to COURSE_VIDEO_SHARING_PER_VIDEO or None / unset - else: - return self.public_access - def is_transcript_feedback_enabled(self): """ Is transcript feedback enabled for this video? @@ -571,7 +512,8 @@ class _BuiltInVideoBlock( """ Returns the public video url """ - return fr'{settings.LMS_ROOT_URL}/videos/{str(self.location)}' + video_config_service = self.runtime.service(self, 'video_config') + return video_config_service.get_public_video_url(self) if video_config_service else None def validate(self): """