feat: course overrides of video sharing settings (#32169)

* refactor: public sharing enabled toggle

Cherry-picked from https://github.com/openedx/edx-platform/pull/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.

lms/djangoapps/courseware/tests/test_video_mongo.py

@@ -3,6 +3,7 @@ Video xmodule tests in mongo.
 """
 
 
+from contextlib import contextmanager
 import json
 import shutil
 from collections import OrderedDict
@@ -15,6 +16,7 @@ import pytest
 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
@@ -35,6 +37,11 @@ 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 (
+    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
@@ -135,7 +142,6 @@ class TestVideoYouTube(TestVideo):  # lint-amnesty, pylint: disable=missing-clas
                 {'display_name': 'Text (.txt) file', 'value': 'txt'}
             ],
             'poster': 'null',
-            'public_video_url': None,
         }
 
         mako_service = self.block.runtime.service(self.block, 'mako')
@@ -219,7 +225,6 @@ class TestVideoNonYouTube(TestVideo):  # pylint: disable=test-inherits-tests
                 {'display_name': 'Text (.txt) file', 'value': 'txt'}
             ],
             'poster': 'null',
-            'public_video_url': None,
         }
 
         mako_service = self.block.runtime.service(self.block, 'mako')
@@ -240,6 +245,11 @@ 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),
@@ -247,16 +257,87 @@ class TestVideoPublicAccess(BaseTestVideoXBlock):
         (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
@@ -389,7 +470,6 @@ class TestGetHtmlMethod(BaseTestVideoXBlock):
                 {'display_name': 'Text (.txt) file', 'value': 'txt'}
             ],
             'poster': 'null',
-            'public_video_url': None,
         }
 
         for data in cases:
@@ -510,7 +590,6 @@ class TestGetHtmlMethod(BaseTestVideoXBlock):
                 {'display_name': 'Text (.txt) file', 'value': 'txt'}
             ],
             'poster': 'null',
-            'public_video_url': None,
         }
         initial_context['metadata']['duration'] = None
 
@@ -637,8 +716,7 @@ class TestGetHtmlMethod(BaseTestVideoXBlock):
                 {'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
@@ -811,7 +889,6 @@ class TestGetHtmlMethod(BaseTestVideoXBlock):
                 {'display_name': 'Text (.txt) file', 'value': 'txt'}
             ],
             'poster': 'null',
-            'public_video_url': None,
             'metadata': metadata,
         }
 
@@ -925,7 +1002,6 @@ class TestGetHtmlMethod(BaseTestVideoXBlock):
                 {'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
@@ -1022,7 +1098,6 @@ class TestGetHtmlMethod(BaseTestVideoXBlock):
                 {'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
@@ -2309,7 +2384,6 @@ class TestVideoWithBumper(TestVideo):  # pylint: disable=test-inherits-tests
                 {'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'
@@ -2391,7 +2465,6 @@ class TestAutoAdvanceVideo(TestVideo):  # lint-amnesty, pylint: disable=test-inh
                 {'display_name': 'SubRip (.srt) file', 'value': 'srt'},
                 {'display_name': 'Text (.txt) file', 'value': 'txt'}
             ],
-            'public_video_url': None,
             'poster': 'null'
         }
         return context

lms/djangoapps/courseware/views/views.py

@@ -1772,7 +1772,7 @@ class BasePublicVideoXBlockView(View):
             )
 
             # 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

xmodule/video_block/video_block.py

@@ -33,10 +33,16 @@ from xblock.runtime import KvsFieldData
 
 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
@@ -472,26 +478,72 @@ class VideoBlock(
             '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):
+        """
+        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 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
         """
-        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
+        return urljoin(
+            settings.LMS_ROOT_URL,
+            reverse(
+                'render_public_video_xblock',
+                kwargs={
+                    'usage_key_string': str(self.location)
+                }
+            )
+        )
 
     def validate(self):
         """