From 73c2b1be7e7b9df648dd086c38c03cdee1ca6cfe Mon Sep 17 00:00:00 2001 From: Ahtisham Shahid Date: Thu, 23 Jun 2022 11:48:44 +0500 Subject: [PATCH] Added Big Blue button as live provider (#30613) * feat: added new live provider and fixed tests * feat: added free_tier compatiblity * fix: resolved linter issues and other refactors * fix: ran makemigration to generate migrations * fix: Implemeted key retrival for tabs Co-authored-by: AhtishamShahid --- cms/envs/common.py | 3 + cms/envs/production.py | 7 + cms/envs/test.py | 6 + lms/envs/common.py | 3 + lms/envs/production.py | 7 + lms/envs/test.py | 7 + .../migrations/0002_auto_20220617_1822.py | 23 +++ openedx/core/djangoapps/course_live/models.py | 15 +- .../core/djangoapps/course_live/providers.py | 155 ++++++++++++++++++ .../djangoapps/course_live/serializers.py | 59 ++++--- openedx/core/djangoapps/course_live/tab.py | 25 ++- .../course_live/tests/test_views.py | 103 +++++++++--- openedx/core/djangoapps/course_live/utils.py | 14 -- openedx/core/djangoapps/course_live/views.py | 17 +- 14 files changed, 365 insertions(+), 79 deletions(-) create mode 100644 openedx/core/djangoapps/course_live/migrations/0002_auto_20220617_1822.py create mode 100644 openedx/core/djangoapps/course_live/providers.py delete mode 100644 openedx/core/djangoapps/course_live/utils.py diff --git a/cms/envs/common.py b/cms/envs/common.py index d932f61c47..ce211a29ea 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -2663,3 +2663,6 @@ TEXTBOOKS_HELP_URL = "https://edx.readthedocs.io/projects/open-edx-building-and- WIKI_HELP_URL = "https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/latest/course_assets/course_wiki.html" CUSTOM_PAGES_HELP_URL = "https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/latest/course_assets/pages.html#adding-custom-pages" COURSE_LIVE_HELP_URL = "https://edx.readthedocs.io/projects/edx-partner-course-staff/en/latest/course_assets/course_live.html" + +# keys for big blue button live provider +COURSE_LIVE_GLOBAL_CREDENTIALS = {} diff --git a/cms/envs/production.py b/cms/envs/production.py index 8a20a6ba12..72a130264a 100644 --- a/cms/envs/production.py +++ b/cms/envs/production.py @@ -633,3 +633,10 @@ DISCUSSIONS_MFE_FEEDBACK_URL = ENV_TOKENS.get('DISCUSSIONS_MFE_FEEDBACK_URL', DI ############## DRF overrides ############## REST_FRAMEWORK.update(ENV_TOKENS.get('REST_FRAMEWORK', {})) + +# keys for big blue button live provider +COURSE_LIVE_GLOBAL_CREDENTIALS["BIG_BLUE_BUTTON"] = { + "KEY": ENV_TOKENS.get('BIG_BLUE_BUTTON_GLOBAL_KEY', None), + "SECRET": ENV_TOKENS.get('BIG_BLUE_BUTTON_GLOBAL_SECRET', None), + "URL": ENV_TOKENS.get('BIG_BLUE_BUTTON_GLOBAL_URL', None), +} diff --git a/cms/envs/test.py b/cms/envs/test.py index a42b73336a..c31635d6bf 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -362,3 +362,9 @@ PROCTORING_USER_OBFUSCATION_KEY = 'test_key' #################### Network configuration #################### # Tests are not behind any proxies CLOSEST_CLIENT_IP_FROM_HEADERS = [] + +COURSE_LIVE_GLOBAL_CREDENTIALS["BIG_BLUE_BUTTON"] = { + "KEY": "***", + "SECRET": "***", + "URL": "***", +} diff --git a/lms/envs/common.py b/lms/envs/common.py index 8b9a4b1528..6a9ee67479 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -5129,3 +5129,6 @@ CREATE_FINANCIAL_ASSISTANCE_APPLICATION_URL = '/core/api/financial_assistance_ap ENTERPRISE_BACKEND_SERVICE_EDX_OAUTH2_KEY = "enterprise-backend-service-key" ENTERPRISE_BACKEND_SERVICE_EDX_OAUTH2_SECRET = "enterprise-backend-service-secret" ENTERPRISE_BACKEND_SERVICE_EDX_OAUTH2_PROVIDER_URL = "http://127.0.0.1:8000/oauth2" + +# keys for big blue button live provider +COURSE_LIVE_GLOBAL_CREDENTIALS = {} diff --git a/lms/envs/production.py b/lms/envs/production.py index 680c261a04..fabcc024ef 100644 --- a/lms/envs/production.py +++ b/lms/envs/production.py @@ -1071,3 +1071,10 @@ REST_FRAMEWORK.update(ENV_TOKENS.get('REST_FRAMEWORK', {})) ############################# CELERY ############################ CELERY_IMPORTS.extend(ENV_TOKENS.get('CELERY_EXTRA_IMPORTS', [])) + +# keys for big blue button live provider +COURSE_LIVE_GLOBAL_CREDENTIALS["BIG_BLUE_BUTTON"] = { + "KEY": ENV_TOKENS.get('BIG_BLUE_BUTTON_GLOBAL_KEY', None), + "SECRET": ENV_TOKENS.get('BIG_BLUE_BUTTON_GLOBAL_SECRET', None), + "URL": ENV_TOKENS.get('BIG_BLUE_BUTTON_GLOBAL_URL', None), +} diff --git a/lms/envs/test.py b/lms/envs/test.py index 61cece154a..edef277574 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -640,3 +640,10 @@ SAVE_FOR_LATER_EMAIL_RATE_LIMIT = '5/m' #################### Network configuration #################### # Tests are not behind any proxies CLOSEST_CLIENT_IP_FROM_HEADERS = [] + + +COURSE_LIVE_GLOBAL_CREDENTIALS["BIG_BLUE_BUTTON"] = { + "KEY": "***", + "SECRET": "***", + "URL": "***", +} diff --git a/openedx/core/djangoapps/course_live/migrations/0002_auto_20220617_1822.py b/openedx/core/djangoapps/course_live/migrations/0002_auto_20220617_1822.py new file mode 100644 index 0000000000..7411625cbd --- /dev/null +++ b/openedx/core/djangoapps/course_live/migrations/0002_auto_20220617_1822.py @@ -0,0 +1,23 @@ +# Generated by Django 3.2.13 on 2022-06-17 18:22 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('course_live', '0001_initial'), + ] + + operations = [ + migrations.AddField( + model_name='courseliveconfiguration', + name='free_tier', + field=models.BooleanField(default=False, help_text='True, if LTI credential are provided by Org globally'), + ), + migrations.AddField( + model_name='historicalcourseliveconfiguration', + name='free_tier', + field=models.BooleanField(default=False, help_text='True, if LTI credential are provided by Org globally'), + ), + ] diff --git a/openedx/core/djangoapps/course_live/models.py b/openedx/core/djangoapps/course_live/models.py index 677f362fc5..a871ec0244 100644 --- a/openedx/core/djangoapps/course_live/models.py +++ b/openedx/core/djangoapps/course_live/models.py @@ -8,17 +8,6 @@ from model_utils.models import TimeStampedModel from opaque_keys.edx.django.models import CourseKeyField from simple_history.models import HistoricalRecords -AVAILABLE_PROVIDERS = { - 'zoom': { - 'name': 'Zoom LTI PRO', - 'features': [], - 'pii_sharing': { - 'username': False, - 'email': False, - } - } -} - class CourseLiveConfiguration(TimeStampedModel): """ @@ -42,6 +31,10 @@ class CourseLiveConfiguration(TimeStampedModel): verbose_name=_("LTI provider"), help_text=_("The LTI provider's id"), ) + free_tier = models.BooleanField( + default=False, + help_text=_("True, if LTI credential are provided by Org globally") + ) history = HistoricalRecords() def __str__(self): diff --git a/openedx/core/djangoapps/course_live/providers.py b/openedx/core/djangoapps/course_live/providers.py new file mode 100644 index 0000000000..4c0c0af4d4 --- /dev/null +++ b/openedx/core/djangoapps/course_live/providers.py @@ -0,0 +1,155 @@ +""" +LTI Providers for course live module +""" +from abc import ABC +from typing import List, Dict +from django.conf import settings + + +class LiveProvider(ABC): + """ + Defines basic structure of lti provider + """ + id: str + name: str + features: List[str] = [] + requires_username: bool = False + requires_email: bool = False + additional_parameters: List[str] = [] + + @property + def has_free_tier(self) -> bool: + """ + Property defines if provider has free tier + """ + return False + + def requires_pii_sharing(self): + """ + Check if provider requires any PII ie username or email + """ + return self.requires_email or self.requires_username + + def requires_custom_email(self): + """ + Check if provider requires custom instructor email + """ + return 'custom_instructor_email' in self.additional_parameters + + @property + def is_enabled(self): + """ + To check if provider is enabled + To be implemented in subclasses + """ + raise NotImplementedError() + + def __dict__(self): + return { + 'name': self.name, + 'has_free_tier': self.has_free_tier, + 'features': self.features, + 'pii_sharing': { + 'username': self.requires_username, + 'email': self.requires_email, + }, + 'additional_parameters': self.additional_parameters + } + + +class HasGlobalCredentials(ABC): + """ + Defines structure for providers with global credentials + """ + key: str + secret: str + url: str + + @staticmethod + def get_global_keys() -> Dict: + """ + Get keys from settings + """ + raise NotImplementedError() + + def has_valid_global_keys(self) -> bool: + """ + Check if keys are valid and not None + """ + raise NotImplementedError() + + +class Zoom(LiveProvider): + """ + Zoom LTI PRO live provider + """ + id = 'zoom' + name = 'Zoom LTI PRO' + additional_parameters = [ + 'custom_instructor_email' + ] + + @property + def is_enabled(self): + return True + + +class BigBlueButton(LiveProvider, HasGlobalCredentials): + """ + Big Blue Button LTI provider + """ + id = 'big_blue_button' + name = 'Big Blue Button' + requires_username: bool = True + + @property + def has_free_tier(self) -> bool: + """ + Check if free tier is enabled by checking for valid keys + """ + return self.has_valid_global_keys() + + @property + def is_enabled(self) -> bool: + return True + + @staticmethod + def get_global_keys() -> Dict: + """ + Get keys from settings + """ + try: + return settings.COURSE_LIVE_GLOBAL_CREDENTIALS.get('BIG_BLUE_BUTTON', {}) + except AttributeError: + return {} + + def has_valid_global_keys(self) -> bool: + """ + Check if keys are valid and not None + """ + credentials = self.get_global_keys() + if credentials: + self.key = credentials['KEY'] + self.secret = credentials['SECRET'] + self.url = credentials['URL'] + return bool(credentials.get("KEY", None) + and credentials.get("SECRET", None) + and credentials.get("URL", None)) + return False + + +class ProviderManager: + """ + This class provides access to all available provider objects + """ + providers: Dict[str, LiveProvider] + + def __init__(self): + # auto detect live providers. + self.providers = {provider.id: provider() for provider in LiveProvider.__subclasses__()} + + def get_enabled_providers(self) -> Dict[str, LiveProvider]: + """ + Get Enabled providers + """ + return {key: provider for (key, provider) in self.providers.items() if provider.is_enabled} diff --git a/openedx/core/djangoapps/course_live/serializers.py b/openedx/core/djangoapps/course_live/serializers.py index be0d5c4ab6..9fa3405fac 100644 --- a/openedx/core/djangoapps/course_live/serializers.py +++ b/openedx/core/djangoapps/course_live/serializers.py @@ -6,7 +6,11 @@ from django.core.validators import validate_email from lti_consumer.models import LtiConfiguration from rest_framework import serializers -from .models import AVAILABLE_PROVIDERS, CourseLiveConfiguration +from .models import CourseLiveConfiguration +# from .utils import provider_requires_custom_email +from .providers import ProviderManager + +providers = ProviderManager().get_enabled_providers() class LtiSerializer(serializers.ModelSerializer): @@ -34,12 +38,18 @@ class LtiSerializer(serializers.ModelSerializer): """ additional_parameters = value.get('additional_parameters', None) custom_instructor_email = additional_parameters.get('custom_instructor_email', None) - if additional_parameters and custom_instructor_email: + requires_email = self.context.get('provider').requires_custom_email() + + if additional_parameters and custom_instructor_email and requires_email: try: validate_email(custom_instructor_email) except ValidationError as error: raise serializers.ValidationError(f'{custom_instructor_email} is not valid email address') from error return value + + if not requires_email: + return value + raise serializers.ValidationError('custom_instructor_email is required value in additional_parameters') def create(self, validated_data): @@ -75,9 +85,7 @@ class LtiSerializer(serializers.ModelSerializer): if key in self.Meta.fields: setattr(instance, key, value) - share_email, share_username = self.pii_sharing_allowed() - instance.pii_share_username = share_username - instance.pii_share_email = share_email + instance.pii_share_email, instance.pii_share_username = self.pii_sharing_allowed() instance.save() return instance @@ -86,12 +94,10 @@ class LtiSerializer(serializers.ModelSerializer): Check if email and username sharing is required and allowed """ pii_sharing_allowed = self.context.get('pii_sharing_allowed', False) - provider = AVAILABLE_PROVIDERS.get(self.context.get('provider_type', None)) - - email = pii_sharing_allowed and provider['pii_sharing']['email'] if provider else False - username = pii_sharing_allowed and provider['pii_sharing']['username'] if provider else False - - return email, username + provider = self.context.get('provider') + if pii_sharing_allowed and provider: + return provider.requires_email, provider.requires_username + return False, False class CourseLiveConfigurationSerializer(serializers.ModelSerializer): @@ -104,15 +110,27 @@ class CourseLiveConfigurationSerializer(serializers.ModelSerializer): class Meta: model = CourseLiveConfiguration - fields = ['course_key', 'provider_type', 'enabled', 'lti_configuration', 'pii_sharing_allowed'] + fields = ['course_key', 'provider_type', 'enabled', 'lti_configuration', 'pii_sharing_allowed', 'free_tier'] read_only_fields = ['course_key'] + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + try: + self.context['provider_type'] = self.initial_data.get('provider_type', '') + except AttributeError: + self.context['provider_type'] = self.data.get('provider_type', '') + + def validate_free_tier(self, value): + if value == self.context['provider'].has_free_tier: + return value + raise serializers.ValidationError('Provider does not support free tier') + def get_pii_sharing_allowed(self, instance): return self.context['pii_sharing_allowed'] def to_representation(self, instance): payload = super().to_representation(instance) - if not payload['lti_configuration']: + if not payload['lti_configuration'] and not instance.free_tier: payload['lti_configuration'] = LtiSerializer(LtiConfiguration()).data return payload @@ -123,7 +141,8 @@ class CourseLiveConfigurationSerializer(serializers.ModelSerializer): lti_config = validated_data.pop('lti_configuration') instance = CourseLiveConfiguration() instance = self._update_course_live_instance(instance, validated_data) - instance = self._update_lti(instance, lti_config) + if not validated_data.get('free_tier', False): + instance = self._update_lti(instance, lti_config) instance.save() return instance @@ -133,7 +152,8 @@ class CourseLiveConfigurationSerializer(serializers.ModelSerializer): """ lti_config = validated_data.pop('lti_configuration') instance = self._update_course_live_instance(instance, validated_data) - instance = self._update_lti(instance, lti_config) + if not validated_data.get('free_tier', False): + instance = self._update_lti(instance, lti_config) instance.save() return instance @@ -143,9 +163,9 @@ class CourseLiveConfigurationSerializer(serializers.ModelSerializer): """ instance.course_key = self.context.get('course_id') instance.enabled = self.validated_data.get('enabled', False) - - if data.get('provider_type') in AVAILABLE_PROVIDERS: - instance.provider_type = data.get('provider_type') + instance.free_tier = self.validated_data.get('free_tier', False) + if provider := self.context.get('provider'): + instance.provider_type = provider.id else: raise serializers.ValidationError( f'Provider type {data.get("provider_type")} does not exist') @@ -159,14 +179,13 @@ class CourseLiveConfigurationSerializer(serializers.ModelSerializer): """ Update LtiConfiguration """ - lti_serializer = LtiSerializer( instance.lti_configuration or None, data=lti_config, partial=True, context={ 'pii_sharing_allowed': self.context.get('pii_sharing_allowed', False), - 'provider_type': self.context.get('provider_type', ''), + 'provider': self.context.get('provider'), } ) if lti_serializer.is_valid(raise_exception=True): diff --git a/openedx/core/djangoapps/course_live/tab.py b/openedx/core/djangoapps/course_live/tab.py index 27b8e5ff29..3f93c7fa89 100644 --- a/openedx/core/djangoapps/course_live/tab.py +++ b/openedx/core/djangoapps/course_live/tab.py @@ -9,6 +9,7 @@ from xmodule.tabs import TabFragmentViewMixin from lms.djangoapps.courseware.tabs import EnrolledTab from openedx.core.djangoapps.course_live.config.waffle import ENABLE_COURSE_LIVE from openedx.core.djangoapps.course_live.models import CourseLiveConfiguration +from openedx.core.djangoapps.course_live.providers import ProviderManager, HasGlobalCredentials from openedx.core.lib.cache_utils import request_cached from openedx.features.course_experience.url_helpers import get_learning_mfe_home_url from openedx.features.lti_course_tab.tab import LtiCourseLaunchMixin @@ -41,7 +42,21 @@ class CourseLiveTab(LtiCourseLaunchMixin, TabFragmentViewMixin, EnrolledTab): """ Get course live configurations """ - return CourseLiveConfiguration.get(course.id).lti_configuration + course_live_configurations = CourseLiveConfiguration.get(course.id) + if course_live_configurations.free_tier: + providers = ProviderManager().get_enabled_providers() + provider = providers[course_live_configurations.provider_type] + if isinstance(provider, HasGlobalCredentials): + return LtiConfiguration( + lti_1p1_launch_url=provider.url, + lti_1p1_client_key=provider.key, + lti_1p1_client_secret=provider.secret, + version='lti_1p1', + config_store=LtiConfiguration.CONFIG_ON_DB, + ) + else: + raise ValueError("Provider does not support global credentials") + return course_live_configurations.lti_configuration @classmethod @request_cached() @@ -49,6 +64,8 @@ class CourseLiveTab(LtiCourseLaunchMixin, TabFragmentViewMixin, EnrolledTab): """ Check if the tab is enabled. """ - return (ENABLE_COURSE_LIVE.is_enabled(course.id) and - super().is_enabled(course, user) and - CourseLiveConfiguration.is_enabled(course.id)) + return ( + ENABLE_COURSE_LIVE.is_enabled(course.id) and + super().is_enabled(course, user) and + CourseLiveConfiguration.is_enabled(course.id) + ) diff --git a/openedx/core/djangoapps/course_live/tests/test_views.py b/openedx/core/djangoapps/course_live/tests/test_views.py index 821491bab9..6a67ab6634 100644 --- a/openedx/core/djangoapps/course_live/tests/test_views.py +++ b/openedx/core/djangoapps/course_live/tests/test_views.py @@ -2,7 +2,7 @@ Test for course live app views """ import json - +import ddt from django.test import RequestFactory from django.urls import reverse from edx_toggles.toggles.testutils import override_waffle_flag @@ -14,9 +14,13 @@ from xmodule.modulestore.tests.django_utils import CourseUserType, ModuleStoreTe from xmodule.modulestore.tests.factories import CourseFactory from ..config.waffle import ENABLE_COURSE_LIVE -from ..models import AVAILABLE_PROVIDERS, CourseLiveConfiguration +from ..models import CourseLiveConfiguration +from ..providers import ProviderManager + +providers = ProviderManager().get_enabled_providers() +@ddt.ddt class TestCourseLiveConfigurationView(ModuleStoreTestCase, APITestCase): """ Unit tests for the CourseLiveConfigurationView. @@ -42,11 +46,13 @@ class TestCourseLiveConfigurationView(ModuleStoreTestCase, APITestCase): def _post(self, data): return self.client.post(self.url, data, format='json') - def create_course_live_config(self): + def create_course_live_config(self, provider='zoom'): """ creates a courseLiveConfiguration """ - CourseAllowPIISharingInLTIFlag.objects.create(course_id=self.course.id, enabled=True) + if providers.get(provider).requires_pii_sharing(): + CourseAllowPIISharingInLTIFlag.objects.create(course_id=self.course.id, enabled=True) + lti_config = { 'lti_1p1_client_key': 'this_is_key', 'lti_1p1_client_secret': 'this_is_secret', @@ -59,7 +65,7 @@ class TestCourseLiveConfigurationView(ModuleStoreTestCase, APITestCase): } course_live_config_data = { 'enabled': True, - 'provider_type': 'zoom', + 'provider_type': provider, 'lti_configuration': lti_config } response = self._post(course_live_config_data) @@ -82,6 +88,7 @@ class TestCourseLiveConfigurationView(ModuleStoreTestCase, APITestCase): 'version': 'lti_1p1', 'lti_config': {} }, + 'free_tier': False, 'pii_sharing_allowed': False } self.assertEqual(response.data, expected_data) @@ -104,16 +111,19 @@ class TestCourseLiveConfigurationView(ModuleStoreTestCase, APITestCase): 'lti_config': {}, 'version': 'lti_1p1' }, + 'free_tier': False, 'provider_type': '' } content = json.loads(response.content.decode('utf-8')) self.assertEqual(content, expected_data) - def test_create_configurations_data(self): + @ddt.data(('zoom', False, False), ('big_blue_button', False, True)) + @ddt.unpack + def test_create_configurations_data(self, provider, share_email, share_username): """ Create and test courseLiveConfiguration data in database """ - lti_config, data, response = self.create_course_live_config() + lti_config, data, response = self.create_course_live_config(provider) course_live_configurations = CourseLiveConfiguration.get(self.course.id) lti_configuration = CourseLiveConfiguration.get(self.course.id).lti_configuration @@ -125,49 +135,55 @@ class TestCourseLiveConfigurationView(ModuleStoreTestCase, APITestCase): self.assertEqual(lti_config['lti_1p1_client_secret'], lti_configuration.lti_1p1_client_secret) self.assertEqual(lti_config['lti_1p1_launch_url'], lti_configuration.lti_1p1_launch_url) self.assertEqual({ - 'pii_share_username': False, - 'pii_share_email': False, + 'pii_share_username': share_username, + 'pii_share_email': share_email, 'additional_parameters': {'custom_instructor_email': 'email@example.com'} }, lti_configuration.lti_config) self.assertEqual(response.status_code, 200) - def test_create_configurations_response(self): + @ddt.data(('zoom', False, False), ('big_blue_button', False, True)) + @ddt.unpack + def test_create_configurations_response(self, provider, share_email, share_username): """ Create and test POST request response data """ - lti_config, course_live_config_data, response = self.create_course_live_config() + lti_config, course_live_config_data, response = self.create_course_live_config(provider) expected_data = { 'course_key': str(self.course.id), 'enabled': True, - 'pii_sharing_allowed': True, - 'provider_type': 'zoom', + 'pii_sharing_allowed': share_email or share_username, + 'provider_type': provider, + 'free_tier': False, 'lti_configuration': { 'lti_1p1_client_key': 'this_is_key', 'lti_1p1_client_secret': 'this_is_secret', 'lti_1p1_launch_url': 'example.com', 'version': 'lti_1p1', 'lti_config': { - 'pii_share_email': False, - 'pii_share_username': False, + 'pii_share_email': share_email, + 'pii_share_username': share_username, 'additional_parameters': { 'custom_instructor_email': 'email@example.com' }, }, }, } + content = json.loads(response.content.decode('utf-8')) self.assertEqual(response.status_code, 200) self.assertEqual(content, expected_data) - def test_update_configurations_response(self): + @ddt.data(('zoom', False, False), ('big_blue_button', False, True)) + @ddt.unpack + def test_update_configurations_response(self, provider, share_email, share_username): """ Create, update & test POST request response data """ - self.create_course_live_config() + self.create_course_live_config(provider) updated_data = { 'enabled': False, - 'provider_type': 'zoom', + 'provider_type': provider, 'lti_configuration': { 'lti_1p1_client_key': 'new_key', 'lti_1p1_client_secret': 'new_secret', @@ -184,23 +200,24 @@ class TestCourseLiveConfigurationView(ModuleStoreTestCase, APITestCase): self.assertEqual(response.status_code, 200) expected_data = { 'course_key': str(self.course.id), - 'provider_type': 'zoom', + 'provider_type': provider, 'enabled': False, + 'free_tier': False, 'lti_configuration': { 'lti_1p1_client_key': 'new_key', 'lti_1p1_client_secret': 'new_secret', 'lti_1p1_launch_url': 'example01.com', 'version': 'lti_1p1', 'lti_config': { - 'pii_share_username': False, - 'pii_share_email': False, + 'pii_share_username': share_username, + 'pii_share_email': share_email, 'additional_parameters': { 'custom_instructor_email': 'new_email@example.com' } } }, - 'pii_sharing_allowed': True + 'pii_sharing_allowed': share_email or share_username } self.assertEqual(content, expected_data) @@ -249,6 +266,46 @@ class TestCourseLiveConfigurationView(ModuleStoreTestCase, APITestCase): 'url': f'http://learning-mfe/course/{self.course.id}/live' }) + @ddt.data(('big_blue_button', False, True)) + @ddt.unpack + def test_create_configurations_response_free_tier(self, provider, share_email, share_username): + """ + Create and test POST request response data + """ + if providers.get(provider).requires_pii_sharing(): + CourseAllowPIISharingInLTIFlag.objects.create(course_id=self.course.id, enabled=True) + + lti_config = { + 'lti_1p1_client_key': 'this_is_key', + 'lti_1p1_client_secret': 'this_is_secret', + 'lti_1p1_launch_url': 'example.com', + 'lti_config': { + 'additional_parameters': { + 'custom_instructor_email': "email@example.com" + } + }, + } + course_live_config_data = { + 'free_tier': True, + 'enabled': True, + 'provider_type': provider, + 'lti_configuration': lti_config + } + response = self._post(course_live_config_data) + + expected_data = { + 'course_key': str(self.course.id), + 'enabled': True, + 'pii_sharing_allowed': share_email or share_username, + 'provider_type': provider, + 'free_tier': True, + 'lti_configuration': None + } + + content = json.loads(response.content.decode('utf-8')) + self.assertEqual(response.status_code, 200) + self.assertEqual(content, expected_data) + class TestCourseLiveProvidersView(ModuleStoreTestCase, APITestCase): """ @@ -274,7 +331,7 @@ class TestCourseLiveProvidersView(ModuleStoreTestCase, APITestCase): expected_data = { 'providers': { 'active': '', - 'available': AVAILABLE_PROVIDERS + 'available': {key: provider.__dict__() for (key, provider) in providers.items()} } } response = self.client.get(self.url) diff --git a/openedx/core/djangoapps/course_live/utils.py b/openedx/core/djangoapps/course_live/utils.py deleted file mode 100644 index cfb665c6b9..0000000000 --- a/openedx/core/djangoapps/course_live/utils.py +++ /dev/null @@ -1,14 +0,0 @@ -""" -Util functions for course live app -""" -from openedx.core.djangoapps.course_live.models import AVAILABLE_PROVIDERS - - -def provider_requires_pii_sharing(provider_type): - """ - Check if provider requires any PII ie username or email - """ - provider = AVAILABLE_PROVIDERS.get(provider_type, None) - if provider: - return provider['pii_sharing']['email'] or provider['pii_sharing']['username'] - return False diff --git a/openedx/core/djangoapps/course_live/views.py b/openedx/core/djangoapps/course_live/views.py index 9080726dc0..8cff28b920 100644 --- a/openedx/core/djangoapps/course_live/views.py +++ b/openedx/core/djangoapps/course_live/views.py @@ -19,10 +19,9 @@ from lms.djangoapps.courseware.courses import get_course_with_access from openedx.core.djangoapps.course_live.permissions import IsEnrolledOrStaff, IsStaffOrInstructor from openedx.core.djangoapps.course_live.tab import CourseLiveTab from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser -from .utils import provider_requires_pii_sharing - +from .providers import ProviderManager from ...lib.api.view_utils import verify_course_exists -from .models import AVAILABLE_PROVIDERS, CourseLiveConfiguration +from .models import CourseLiveConfiguration from .serializers import CourseLiveConfigurationSerializer @@ -115,7 +114,8 @@ class CourseLiveConfigurationView(APIView): Handle HTTP/POST requests """ pii_sharing_allowed = get_lti_pii_sharing_state_for_course(course_id) - if not pii_sharing_allowed and provider_requires_pii_sharing(request.data.get('provider_type', '')): + provider = ProviderManager().get_enabled_providers().get(request.data.get('provider_type', ''), None) + if not pii_sharing_allowed and provider.requires_pii_sharing(): return Response({ "pii_sharing_allowed": pii_sharing_allowed, "message": "PII sharing is not allowed on this course" @@ -127,7 +127,8 @@ class CourseLiveConfigurationView(APIView): data=request.data, context={ "pii_sharing_allowed": pii_sharing_allowed, - "course_id": course_id + "course_id": course_id, + "provider": provider } ) if not serializer.is_valid(): @@ -199,10 +200,12 @@ class CourseLiveProvidersView(APIView): Dict: course Live providers """ configuration = CourseLiveConfiguration.get(course_id) + providers = ProviderManager().get_enabled_providers() + selected_provider = providers.get(configuration.provider_type if configuration else None, None) return { "providers": { - "active": configuration.provider_type if configuration else "", - "available": AVAILABLE_PROVIDERS + "active": selected_provider.id if selected_provider else "", + "available": {key: provider.__dict__() for (key, provider) in providers.items()} } }