Skip to content

Commit

Permalink
feat: course overrides of video sharing settings (#32169)
Browse files Browse the repository at this point in the history
* refactor: public sharing enabled toggle

Cherry-picked from #32150

* feat: course video share setting on video block

Adds awareness of course.video_sharing_options setting to video XBlock.
This can override whether or not sharing is enabled for a video or fall
back to per-video settings

* test: course video share setting on video block

Adds awareness of course.video_sharing_options setting to video XBlock.
This can override whether or not sharing is enabled for a video or fall
back to per-video settings

* test: course video share setting on preview page

Extends checking for course override of video share settings to public
video preview page.
  • Loading branch information
nsprenkle authored May 3, 2023
1 parent 4020228 commit 745cda1
Show file tree
Hide file tree
Showing 3 changed files with 157 additions and 32 deletions.
109 changes: 91 additions & 18 deletions lms/djangoapps/courseware/tests/test_video_mongo.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"""


from contextlib import contextmanager
import json
import shutil
from collections import OrderedDict
Expand All @@ -15,6 +16,7 @@
from django.conf import settings
from django.core.files import File
from django.core.files.base import ContentFile
from django.http import Http404
from django.test import TestCase
from django.test.utils import override_settings
from edx_toggles.toggles.testutils import override_waffle_flag
Expand All @@ -35,6 +37,11 @@
from lxml import etree
from path import Path as path
from xmodule.contentstore.content import StaticContent
from xmodule.course_block import (
COURSE_VIDEO_SHARING_ALL_VIDEOS,
COURSE_VIDEO_SHARING_NONE,
COURSE_VIDEO_SHARING_PER_VIDEO
)
from xmodule.exceptions import NotFoundError
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.inheritance import own_metadata
Expand Down Expand Up @@ -135,7 +142,6 @@ def test_video_constructor(self):
{'display_name': 'Text (.txt) file', 'value': 'txt'}
],
'poster': 'null',
'public_video_url': None,
}

mako_service = self.block.runtime.service(self.block, 'mako')
Expand Down Expand Up @@ -219,7 +225,6 @@ def test_video_constructor(self):
{'display_name': 'Text (.txt) file', 'value': 'txt'}
],
'poster': 'null',
'public_video_url': None,
}

mako_service = self.block.runtime.service(self.block, 'mako')
Expand All @@ -240,23 +245,99 @@ class TestVideoPublicAccess(BaseTestVideoXBlock):
}
METADATA = {}

@contextmanager
def mock_feature_toggle(self, enabled=True):
with patch.object(PUBLIC_VIDEO_SHARE, 'is_enabled', return_value=enabled):
yield

@ddt.data(
(True, False),
(False, False),
(False, True),
(True, True),
)
@ddt.unpack
def test_public_video_url(self, is_lms_platform, enable_public_share):
def test_is_public_sharing_enabled(self, is_studio, feature_enabled):
"""Test public video url."""
assert self.block.public_access is True
if not is_lms_platform:
if is_studio:
self.block.runtime.is_author_mode = True
with patch.object(PUBLIC_VIDEO_SHARE, 'is_enabled', return_value=enable_public_share):
context = self.block.render(STUDENT_VIEW).content
# public video url iif PUBLIC_VIDEO_SHARE waffle and is_lms_platform, public_access are true
assert bool(get_context_dict_from_string(context)['public_video_url']) \
is (is_lms_platform and enable_public_share)

with self.mock_feature_toggle(enabled=feature_enabled):
assert self.block.is_public_sharing_enabled() == \
(not is_studio and 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()

@patch('xmodule.video_block.video_block.VideoBlock.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
mock_course_sharing_override.return_value = COURSE_VIDEO_SHARING_ALL_VIDEOS
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()

# 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')
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()

# 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')
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()

# 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')
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()

# I will fall-back to per-video values
self.assertEqual(self.block.public_access, is_public_sharing_enabled)

@ddt.data(False, True)
def test_context(self, is_public_sharing_enabled):
with self.mock_feature_toggle():
with patch.object(
self.block,
'is_public_sharing_enabled',
return_value=is_public_sharing_enabled
):
content = self.block.render(STUDENT_VIEW).content
context = get_context_dict_from_string(content)
assert ('public_sharing_enabled' in context) == is_public_sharing_enabled
assert ('public_video_url' in context) == is_public_sharing_enabled


@ddt.ddt
Expand Down Expand Up @@ -389,7 +470,6 @@ def test_get_html_track(self):
{'display_name': 'Text (.txt) file', 'value': 'txt'}
],
'poster': 'null',
'public_video_url': None,
}

for data in cases:
Expand Down Expand Up @@ -510,7 +590,6 @@ def test_get_html_source(self):
{'display_name': 'Text (.txt) file', 'value': 'txt'}
],
'poster': 'null',
'public_video_url': None,
}
initial_context['metadata']['duration'] = None

Expand Down Expand Up @@ -637,8 +716,7 @@ def test_get_html_with_mocked_edx_video_id(self):
{'display_name': 'Text (.txt) file', 'value': 'txt'}
],
'poster': 'null',
'public_video_url': None,
'metadata': metadata
'metadata': metadata,
}

DATA = SOURCE_XML.format( # lint-amnesty, pylint: disable=invalid-name
Expand Down Expand Up @@ -811,7 +889,6 @@ def helper_get_html_with_edx_video_id(self, data):
{'display_name': 'Text (.txt) file', 'value': 'txt'}
],
'poster': 'null',
'public_video_url': None,
'metadata': metadata,
}

Expand Down Expand Up @@ -925,7 +1002,6 @@ def side_effect(*args, **kwargs): # lint-amnesty, pylint: disable=unused-argume
{'display_name': 'SubRip (.srt) file', 'value': 'srt'},
{'display_name': 'Text (.txt) file', 'value': 'txt'}
],
'public_video_url': None,
'poster': 'null',
}
initial_context['metadata']['duration'] = None
Expand Down Expand Up @@ -1022,7 +1098,6 @@ def test_get_html_cdn_source_external_video(self):
{'display_name': 'SubRip (.srt) file', 'value': 'srt'},
{'display_name': 'Text (.txt) file', 'value': 'txt'}
],
'public_video_url': None,
'poster': 'null',
}
initial_context['metadata']['duration'] = None
Expand Down Expand Up @@ -2309,7 +2384,6 @@ def test_bumper_metadata(self, get_url_for_profiles, get_bumper_settings, is_bum
{'display_name': 'SubRip (.srt) file', 'value': 'srt'},
{'display_name': 'Text (.txt) file', 'value': 'txt'}
],
'public_video_url': None,
'poster': json.dumps(OrderedDict({
'url': 'http://img.youtube.com/vi/ZwkTiUPN0mg/0.jpg',
'type': 'youtube'
Expand Down Expand Up @@ -2391,7 +2465,6 @@ def prepare_expected_context(self, autoadvanceenabled_flag, autoadvance_flag):
{'display_name': 'SubRip (.srt) file', 'value': 'srt'},
{'display_name': 'Text (.txt) file', 'value': 'txt'}
],
'public_video_url': None,
'poster': 'null'
}
return context
Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/courseware/views/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1772,7 +1772,7 @@ def get_course_and_video_block(self, usage_key_string):
)

# Block must be marked as public to be viewed
if not video_block.public_access:
if not video_block.is_public_sharing_enabled():
raise Http404("Video not found.")

return course, video_block
Expand Down
78 changes: 65 additions & 13 deletions xmodule/video_block/video_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,16 @@

from common.djangoapps.xblock_django.constants import ATTR_KEY_REQUEST_COUNTRY_CODE
from openedx.core.djangoapps.video_config.models import HLSPlaybackEnabledFlag, CourseYoutubeBlockedFlag
from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE
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
Expand Down Expand Up @@ -472,26 +478,72 @@ def get_html(self, view=STUDENT_VIEW, context=None): # lint-amnesty, pylint: di
'track': track_url,
'transcript_download_format': transcript_download_format,
'transcript_download_formats_list': self.fields['transcript_download_format'].values, # lint-amnesty, pylint: disable=unsubscriptable-object
# provide the video url iif the video is public and we are in LMS.
# Reverse render_public_video_xblock is not available in studio.
'public_video_url': self._get_public_video_url(),
}

# Public video previewing / social media sharing
if self.is_public_sharing_enabled():
template_context['public_sharing_enabled'] = True
template_context['public_video_url'] = self.get_public_video_url()

return self.runtime.service(self, 'mako').render_template('video.html', template_context)

def _get_public_video_url(self):
def get_course_video_sharing_override(self):
"""
Returns the public video url
Return course video sharing options override or None
"""
try:
course = get_course_by_id(self.course_id)
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?
"""

# Sharing is DISABLED from studio
is_studio = getattr(self.runtime, "is_author_mode", False)
if self.public_access and not is_studio:
from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE
if PUBLIC_VIDEO_SHARE.is_enabled(self.location.course_key):
return urljoin(
settings.LMS_ROOT_URL,
reverse('render_public_video_xblock', kwargs={'usage_key_string': str(self.location)})
)
return None
if is_studio:
return False

# Video share feature must be enabled for sharing settings to take effect
feature_enabled = PUBLIC_VIDEO_SHARE.is_enabled(self.location.course_key)
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 get_public_video_url(self):
"""
Returns the public video url
"""
return urljoin(
settings.LMS_ROOT_URL,
reverse(
'render_public_video_xblock',
kwargs={
'usage_key_string': str(self.location)
}
)
)

def validate(self):
"""
Expand Down

0 comments on commit 745cda1

Please sign in to comment.