Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion cms/djangoapps/contentstore/views/preview.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
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
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions lms/djangoapps/courseware/block_render.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
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
Expand Down Expand Up @@ -635,6 +636,7 @@ def inner_get_block(block: XBlock) -> XBlock | None:
'call_to_action': CallToActionService(),
'publish': EventPublishingService(user, course_id, track_function),
'enrollments': EnrollmentsService(),
'video_config': VideoConfigService(),
}

runtime.get_block_for_descriptor = inner_get_block
Expand Down
25 changes: 13 additions & 12 deletions lms/djangoapps/courseware/tests/test_video_mongo.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
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
Expand All @@ -57,6 +57,7 @@
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
Expand Down Expand Up @@ -260,14 +261,14 @@ def test_is_public_sharing_enabled(self, feature_enabled):
"""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
Expand All @@ -276,47 +277,47 @@ def test_is_public_sharing_enabled_by_course_override(self, mock_course_sharing_

# 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
self.block.public_access = 'some-arbitrary-value'

# 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
self.block.public_access = 'some-arbitrary-value'

# 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()
self.block.public_access = 'some-arbitrary-value'

# 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)
Expand All @@ -325,7 +326,7 @@ def test_is_public_sharing_course_not_found(self, mock_get_course):
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
):
Expand Down
3 changes: 2 additions & 1 deletion lms/djangoapps/courseware/views/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@
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
Expand Down Expand Up @@ -1869,7 +1870,7 @@ def get_course_and_video_block(self, usage_key_string):
)

# 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
Expand Down
64 changes: 64 additions & 0 deletions openedx/core/djangoapps/video_config/services.py
Original file line number Diff line number Diff line change
@@ -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
81 changes: 81 additions & 0 deletions openedx/core/djangoapps/video_config/sharing.py
Original file line number Diff line number Diff line change
@@ -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
8 changes: 5 additions & 3 deletions xmodule/course_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@
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
Expand Down Expand Up @@ -55,9 +60,6 @@
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
Expand Down
6 changes: 5 additions & 1 deletion xmodule/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
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


Expand Down Expand Up @@ -159,6 +160,7 @@ def get_block(block):
'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
Expand Down Expand Up @@ -214,6 +216,7 @@ def get_block(block):
'cache': CacheService(DoNothingCache()),
'field-data': DictFieldData({}),
'sandbox': SandboxService(contentstore, course_id),
'video_config': VideoConfigService(),
}

if add_overrides:
Expand Down Expand Up @@ -241,14 +244,15 @@ 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'),
resources_fs=Mock(name='get_test_descriptor_system.resources_fs'),
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},

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this addition is necessary in order to get tests to pass, then this is fine.

But, if tests pass without the service, then please remove it. It's best to keep the test runtime as simple as possible. For example, there's not even a user service in this runtime.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, without this injection tests are failing.

FAILED lms/djangoapps/courseware/tests/test_video_mongo.py::VideoBlockTest::test_get_context - xblock.exceptions.NoSuchServiceError: Service 'video_config' is not available.

Detailed logs

_________________________________________________________________________________ VideoBlockTest.test_get_context _________________________________________________________________________________

self = <lms.djangoapps.courseware.tests.test_video_mongo.VideoBlockTest testMethod=test_get_context>

    def test_get_context(self):
        """"
        Test get_context.
    
        This test is located here and not in xmodule.tests because get_context calls editable_metadata_fields.
        Which, in turn, uses settings.LANGUAGES from django setttings.
        """
        correct_tabs = [
            {
                'name': "Basic",
                'template': "video/transcripts.html",
                'current': True
            },
            {
                'name': 'Advanced',
                'template': 'tabs/metadata-edit-tab.html'
            }
        ]
>       rendered_context = self.block.get_context()

lms/djangoapps/courseware/tests/test_video_mongo.py:1829: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
xmodule/video_block/video_block.py:849: in get_context
    _context = MakoTemplateBlockBase.get_context(self)
xmodule/mako_block.py:56: in get_context
    'editable_metadata_fields': self.editable_metadata_fields
xmodule/video_block/video_block.py:647: in editable_metadata_fields
    editable_fields['public_access']['url'] = self.get_public_video_url()
xmodule/video_block/video_block.py:523: in get_public_video_url
    video_config_service = self.runtime.service(self, 'video_config')
xmodule/x_module.py:1505: in service
    service = super().service(block=block, service_name=service_name)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <xmodule.tests.TestDescriptorSystem object at 0xffff67fea350>
block = <VideoBlockWithMixins @E3CB license=None, name=None, parent=None, tags=[], display_name='Video', course_edit_method='S...{}, youtube_id_0_75='', youtube_id_1_0='3_yD_cEKoCk', youtube_id_1_25='', youtube_id_1_5='', youtube_is_available=True>
service_name = 'video_config'

    def service(self, block, service_name):
        """Return a service, or None.
    
        Services are objects implementing arbitrary other interfaces.  They are
        requested by agreed-upon names, see [XXX TODO] for a list of possible
        services.  The object returned depends on the service requested.
    
        XBlocks must announce their intention to request services with the
        `XBlock.needs` or `XBlock.wants` decorators.  Use `needs` if you assume
        that the service is available, or `wants` if your code is flexible and
        can accept a None from this method.
    
        Runtimes can override this method if they have different techniques for
        finding and delivering services.
    
        Arguments:
            block (XBlock): this block's class will be examined for service
                decorators.
            service_name (str): the name of the service requested.
    
        Returns:
            An object implementing the requested service, or None.
    
        """
        declaration = block.service_declaration(service_name)
        if declaration is None:
            raise NoSuchServiceError(f"Service {service_name!r} was not requested.")
        service = self._services.get(service_name)
        if service is None and declaration == "need":
>           raise NoSuchServiceError(f"Service {service_name!r} is not available.")
E           xblock.exceptions.NoSuchServiceError: Service 'video_config' is not available.

/mnt/xblock/xblock/runtime.py:1101: NoSuchServiceError

**kwargs
)
descriptor_system.get_asides = lambda block: []
Expand Down
Loading
Loading