Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: Allow fetching details of inactive providers [BD-38] (#29656)

Browse Source 
* feat: Allow fetching details of inactive providers

* squash!: update tests add docs

* squash!: review feedback

* squash!: fix quality issue

* squash!: review feedback
This commit is contained in:
Kshitij Sobti
2022-01-31 10:58:27 +00:00
committed by GitHub
parent 2758301c31
commit 16881afcad
5 changed files with 306 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
openedx/core/djangoapps/discussions/models.py
View File

@@ -169,7 +169,6 @@ AVAILABLE_PROVIDER_MAP = {
'messages': [],
'has_full_support': True,
'supports_in_context_discussions': True,
'visible': False,
},
Provider.ED_DISCUSS: {
'features': [

87
openedx/core/djangoapps/discussions/serializers.py
View File

@@ -9,7 +9,7 @@ from rest_framework import serializers
from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings
from openedx.core.lib.courses import get_course_by_id
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
from .models import AVAILABLE_PROVIDER_MAP, DEFAULT_PROVIDER_TYPE, DiscussionsConfiguration, Features, Provider
from .models import DiscussionsConfiguration, Provider
from .utils import available_division_schemes, get_divided_discussions
@@ -208,41 +208,34 @@ class DiscussionsConfigurationSerializer(serializers.ModelSerializer):
Serialize data into a dictionary, to be used as a response
"""
course_key = instance.context_key
active_provider = instance.provider_type
provider_type = self.context.get('provider_type') or active_provider
payload = super().to_representation(instance)
course_pii_sharing_allowed = get_lti_pii_sharing_state_for_course(course_key)
lti_configuration = LtiSerializer(instance=instance.lti_configuration)
lti_configuration_data = lti_configuration.data
lti_configuration_data.update({
'pii_sharing_allowed': course_pii_sharing_allowed
})
provider_type = instance.provider_type
plugin_configuration = instance.plugin_configuration
# LTI configuration is only stored for the active provider.
if provider_type == active_provider:
lti_configuration = LtiSerializer(instance=instance.lti_configuration)
lti_configuration_data = lti_configuration.data
plugin_configuration = instance.plugin_configuration
else:
lti_configuration_data = {}
plugin_configuration = {}
course = get_course_by_id(course_key)
legacy_settings = LegacySettingsSerializer(
course,
data=plugin_configuration,
)
if legacy_settings.is_valid(raise_exception=True):
plugin_configuration = legacy_settings.data
features_list = [
{'id': feature.value, 'feature_support_type': feature.feature_support_type}
for feature in Features
]
if provider_type in [Provider.LEGACY, Provider.OPEN_EDX]:
legacy_settings = LegacySettingsSerializer(course, data=plugin_configuration)
if legacy_settings.is_valid(raise_exception=True):
plugin_configuration = legacy_settings.data
if provider_type == Provider.OPEN_EDX:
plugin_configuration.update({
"group_at_subsection": instance.plugin_configuration.get("group_at_subsection", False)
})
lti_configuration_data.update({'pii_sharing_allowed': course_pii_sharing_allowed})
payload.update({
'features': features_list,
'provider_type': provider_type,
'lti_configuration': lti_configuration_data,
'plugin_configuration': plugin_configuration,
'providers': {
'active': provider_type or DEFAULT_PROVIDER_TYPE,
'available': {
key: value
for key, value in AVAILABLE_PROVIDER_MAP.items()
if value.get('visible', True)
},
},
})
return payload
@@ -401,7 +394,6 @@ class DiscussionSettingsSerializer(serializers.Serializer):
"""
Return a serialized representation of the course discussion settings.
"""
payload = super().to_representation(instance)
course = self.context['course']
instance = self.context['settings']
course_key = course.id
@@ -434,3 +426,40 @@ class DiscussionSettingsSerializer(serializers.Serializer):
except ValueError as e:
raise ValidationError(str(e)) from e
return instance
class DiscussionsProviderSerializer(serializers.Serializer):
"""
Serializer for a discussion provider
"""
features = serializers.ListField(child=serializers.CharField(), help_text="Features supported by the provider")
supports_lti = serializers.BooleanField(default=False, help_text="Whether the provider supports LTI")
external_links = serializers.DictField(help_text="External documentation and links for provider")
messages = serializers.ListField(child=serializers.CharField(), help_text="Custom messaging for provider")
has_full_support = serializers.BooleanField(help_text="Whether the provider is fully supported")
class DiscussionsFeatureSerializer(serializers.Serializer):
"""
Serializer for discussions features
"""
id = serializers.CharField(help_text="Feature ID")
feature_support_type = serializers.CharField(help_text="Feature support level classification")
class DiscussionsProvidersSerializer(serializers.Serializer):
"""
Serializer for discussion providers.
"""
active = serializers.CharField(
read_only=True,
help_text="The current active provider",
)
features = serializers.ListField(
child=DiscussionsFeatureSerializer(read_only=True),
help_text="Features support classification levels",
)
available = serializers.DictField(
child=DiscussionsProviderSerializer(read_only=True),
help_text="List of available providers",
)

57
openedx/core/djangoapps/discussions/tests/test_views.py
View File

@@ -9,15 +9,18 @@ from datetime import datetime, timedelta, timezone
import ddt
from django.core.exceptions import ValidationError
from django.urls import reverse
from edx_toggles.toggles.testutils import override_waffle_flag
from lti_consumer.models import CourseAllowPIISharingInLTIFlag
from rest_framework import status
from rest_framework.test import APITestCase
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.tests.django_utils import CourseUserType, ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
from common.djangoapps.student.tests.factories import UserFactory
from lms.djangoapps.discussion.django_comment_client.tests.factories import RoleFactory
from lms.djangoapps.discussion.toggles import ENABLE_DISCUSSIONS_MFE
from ..models import AVAILABLE_PROVIDER_MAP, DEFAULT_CONFIG_ENABLED, DEFAULT_PROVIDER_TYPE, Provider
DATA_LEGACY_COHORTS = {
@@ -317,7 +320,6 @@ class DataTest(AuthorizedApiTest):
Provider.INSCRIBE,
Provider.PIAZZA,
Provider.YELLOWDIG,
Provider.LEGACY
)
def test_add_valid_configuration(self, provider_type):
"""
@@ -335,7 +337,7 @@ class DataTest(AuthorizedApiTest):
data = self.get()
assert data['enabled']
assert data['provider_type'] == provider_type
assert data['plugin_configuration'] == DEFAULT_LEGACY_CONFIGURATION
assert data['plugin_configuration'] == {'key': 'value'}
assert data['lti_configuration'] == DEFAULT_LTI_CONFIGURATION
def test_change_plugin_configuration(self):
@@ -352,13 +354,43 @@ class DataTest(AuthorizedApiTest):
}
response = self._post(payload)
data = response.json()
assert data['plugin_configuration'] == DEFAULT_LEGACY_CONFIGURATION
assert data['plugin_configuration'] == {
'allow_anonymous': False,
'custom_field': 'custom_value',
}
course = self.store.get_course(self.course.id)
# Only configuration fields not stored in the course, or
# directly in the model should be stored here.
assert course.discussions_settings['piazza'] == {'custom_field': 'custom_value'}
@ddt.data(
# If the legacy provider is selected show the legacy provider only
# and hide the new provider even if the mfe is enabled
(Provider.LEGACY, [Provider.LEGACY], Provider.OPEN_EDX, False),
(Provider.LEGACY, [Provider.LEGACY], Provider.OPEN_EDX, True),
# If the new provider is selected show the new provider only
# and hide the legacy provider even if the mfe is disabled
(Provider.OPEN_EDX, [Provider.OPEN_EDX], Provider.LEGACY, False),
(Provider.OPEN_EDX, [Provider.OPEN_EDX], Provider.LEGACY, True),
# If some other provider is selected show the new provider
# if the mfe is enabled
(Provider.PIAZZA, [Provider.LEGACY], Provider.OPEN_EDX, False),
(Provider.PIAZZA, [Provider.OPEN_EDX, Provider.LEGACY], 'dummy', True),
)
@ddt.unpack
def test_available_providers_legacy(self, current_provider, visible_providers, hidden_provider, mfe_enabled):
"""
Tests that providers available depending on the course.
"""
self._configure_lti_discussion_provider(provider=current_provider)
with override_waffle_flag(ENABLE_DISCUSSIONS_MFE, mfe_enabled):
response = self._get()
data = response.json()
for visible_provider in visible_providers:
assert visible_provider in data['providers']['available'].keys()
assert hidden_provider not in data['providers']['available'].keys()
@ddt.data(
{'enabled': 3},
)
@@ -503,10 +535,7 @@ class DataTest(AuthorizedApiTest):
data = self.get()
assert data['enabled']
assert data['provider_type'] == Provider.ED_DISCUSS
assert data['plugin_configuration'] == {
**DEFAULT_LEGACY_CONFIGURATION,
'allow_anonymous': False,
}
assert data['plugin_configuration'] == {}
assert data['lti_configuration'] == DATA_LTI_CONFIGURATION_DISABLED_PII
def test_change_from_lti(self):
@@ -727,10 +756,12 @@ class PIISettingsAPITests(DataTest):
"""
data = self._configure_lti_discussion_provider(provider=provider)
self._assert_pii_flag_for_course(enabled=False)
expected_providers = AVAILABLE_PROVIDER_MAP[provider]
expected_providers.pop('admin_only_config', None)
assert data['enabled']
assert data['provider_type'] == provider
assert data['providers']['available'][provider] == AVAILABLE_PROVIDER_MAP[provider]
assert data['plugin_configuration'] == DEFAULT_LEGACY_CONFIGURATION
assert data['providers']['available'][provider] == expected_providers
assert data['plugin_configuration'] == {}
assert data['lti_configuration'] == DATA_LTI_CONFIGURATION_DISABLED_PII
response_data = self.get()
@@ -750,10 +781,12 @@ class PIISettingsAPITests(DataTest):
with self._pii_sharing_for_course(enabled=True):
self._assert_pii_flag_for_course(enabled=True)
data = self._configure_lti_discussion_provider(provider=provider)
expected_providers = AVAILABLE_PROVIDER_MAP[provider]
expected_providers.pop('admin_only_config', None)
assert data['enabled']
assert data['provider_type'] == provider
assert data['providers']['available'][provider] == AVAILABLE_PROVIDER_MAP[provider]
assert data['plugin_configuration'] == DEFAULT_LEGACY_CONFIGURATION
assert data['providers']['available'][provider] == expected_providers
assert data['plugin_configuration'] == {}
assert data['lti_configuration'] == DATA_LTI_CONFIGURATION_ENABLED_PII
response_data = self.get()

21
openedx/core/djangoapps/discussions/urls.py
View File

@@ -1,15 +1,26 @@
"""
Configure URL endpoints for the djangoapp
"""
from django.conf.urls import url
from django.urls import re_path
from django.conf import settings
from .views import DiscussionsConfigurationView
from .views import CombinedDiscussionsConfigurationView, DiscussionsConfigurationSettingsView, DiscussionsProvidersView
urlpatterns = [
url(
r'^v0/(?P<course_key_string>.+)$',
DiscussionsConfigurationView.as_view(),
re_path(
fr'^v0/{settings.COURSE_KEY_PATTERN}$',
CombinedDiscussionsConfigurationView.as_view(),
name='discussions',
),
re_path(
fr'^v0/course/{settings.COURSE_KEY_PATTERN}/settings$',
DiscussionsConfigurationSettingsView.as_view(),
name='discussions-settings',
),
re_path(
fr'^v0/course/{settings.COURSE_KEY_PATTERN}/providers$',
DiscussionsProvidersView.as_view(),
name='discussions-providers',
),
]

196
openedx/core/djangoapps/discussions/views.py
View File

@@ -1,22 +1,31 @@
"""
Handle view-logic for the discussions app.
"""
from typing import Dict
import edx_api_doc_tools as apidocs
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser
from rest_framework.exceptions import ValidationError
from rest_framework.request import Request
from rest_framework.response import Response
from rest_framework.views import APIView
from lms.djangoapps.discussion.toggles import ENABLE_DISCUSSIONS_MFE
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
from openedx.core.lib.api.view_utils import validate_course_key
from .models import DiscussionsConfiguration
from .permissions import check_course_permissions, IsStaffOrCourseTeam
from .serializers import DiscussionsConfigurationSerializer
from .models import AVAILABLE_PROVIDER_MAP, DiscussionsConfiguration, Features, Provider
from .permissions import IsStaffOrCourseTeam, check_course_permissions
from .serializers import (
DiscussionsConfigurationSerializer,
DiscussionsProvidersSerializer,
)
class DiscussionsConfigurationView(APIView):
class DiscussionsConfigurationSettingsView(APIView):
"""
Handle configuration-related view-logic
View for configuring discussion settings.
"""
authentication_classes = (
JwtAuthentication,
@@ -25,25 +34,77 @@ class DiscussionsConfigurationView(APIView):
)
permission_classes = (IsStaffOrCourseTeam,)
# pylint: disable=redefined-builtin
def get(self, request, course_key_string: str, **_kwargs) -> Response:
@apidocs.schema(
parameters=[
apidocs.string_parameter(
'course_id',
apidocs.ParameterLocation.PATH,
description="The course for which to get provider list",
),
apidocs.string_parameter(
'provider_id',
apidocs.ParameterLocation.QUERY,
description="The provider_id to fetch data for"
)
],
responses={
200: DiscussionsConfigurationSerializer,
400: "Invalid provider ID",
401: "The requester is not authenticated.",
403: "The requester cannot access the specified course.",
404: "The requested course does not exist.",
},
)
def get(self, request: Request, course_key_string: str, **_kwargs) -> Response:
"""
Handle HTTP/GET requests
"""
data = self.get_configuration_data(request, course_key_string)
return Response(data)
@staticmethod
def get_configuration_data(request: Request, course_key_string: str) -> Dict:
"""
Get discussions configuration data for the course
Args:
request (Request): a DRF request
course_key_string (str): a course key string
Returns:
Dict: Discussion configuration data for the course
"""
course_key = validate_course_key(course_key_string)
configuration = DiscussionsConfiguration.get(course_key)
provider_type = request.query_params.get('provider_id', None)
if provider_type and provider_type not in AVAILABLE_PROVIDER_MAP:
raise ValidationError("Unsupported provider type")
serializer = DiscussionsConfigurationSerializer(
configuration,
context={
'user_id': request.user.id,
'provider_type': provider_type,
}
)
return Response(serializer.data)
return serializer.data
def post(self, request, course_key_string: str, **_kwargs) -> Response:
"""
Handle HTTP/POST requests
"""
data = self.update_configuration_data(request, course_key_string)
return Response(data)
@staticmethod
def update_configuration_data(request, course_key_string):
"""
Update discussion configuration for the course based on data in the request.
Args:
request (Request): a DRF request
course_key_string (str): a course key string
Returns:
Dict: modified course configuration data
"""
course_key = validate_course_key(course_key_string)
configuration = DiscussionsConfiguration.get(course_key)
course = CourseOverview.get_from_id(course_key)
@@ -61,4 +122,121 @@ class DiscussionsConfigurationView(APIView):
check_course_permissions(course, request.user, 'change_provider')
serializer.save()
return Response(serializer.data)
return serializer.data
class DiscussionsProvidersView(APIView):
"""
Read only view that lists details of discussion providers available for a course.
"""
authentication_classes = (
JwtAuthentication,
BearerAuthenticationAllowInactiveUser,
SessionAuthenticationAllowInactiveUser
)
permission_classes = (IsStaffOrCourseTeam,)
@apidocs.schema(
parameters=[
apidocs.string_parameter(
'course_id',
apidocs.ParameterLocation.PATH,
description="The course for which to get provider list",
)
],
responses={
200: DiscussionsProvidersSerializer,
401: "The requester is not authenticated.",
403: "The requester cannot access the specified course.",
404: "The requested course does not exist.",
},
)
def get(self, request, course_key_string: str, **_kwargs) -> Response:
"""
Handle HTTP/GET requests
"""
data = self.get_provider_data(course_key_string)
return Response(data)
@staticmethod
def get_provider_data(course_key_string: str) -> Dict:
"""
Get provider data for specified course
Args:
course_key_string (str): course key string
Returns:
Dict: course discussion providers
"""
course_key = validate_course_key(course_key_string)
configuration = DiscussionsConfiguration.get(course_key)
hidden_providers = []
# If the user is currently using the legacy provider, don't show the new provider
# TODO: Allow switching between legacy and new providers
if configuration.provider_type == Provider.LEGACY:
hidden_providers.append(Provider.OPEN_EDX)
# If the user is currently using the new provider, don't show the legacy provider
elif configuration.provider_type == Provider.OPEN_EDX:
hidden_providers.append(Provider.LEGACY)
else:
# If this is a new course, or some other provider is selected, the new provider
# should only show up if the MFE is enabled
if not ENABLE_DISCUSSIONS_MFE.is_enabled(course_key):
hidden_providers.append(Provider.OPEN_EDX)
serializer = DiscussionsProvidersSerializer(
{
'features': [
{'id': feature.value, 'feature_support_type': feature.feature_support_type}
for feature in Features
],
'active': configuration.provider_type,
'available': {
key: value
for key, value in AVAILABLE_PROVIDER_MAP.items()
if key not in hidden_providers
},
}
)
return serializer.data
class CombinedDiscussionsConfigurationView(DiscussionsConfigurationSettingsView):
"""
Combined view that includes both provider data and discussion configuration.
Note:
This is temporary code for backwards-compatibility and will be removed soon
after the frontend supports the new split APIs.
"""
def get(self, request: Request, course_key_string: str, **_kwargs) -> Response:
"""
Handle HTTP/GET requests
"""
config_data = self.get_configuration_data(request, course_key_string)
provider_data = DiscussionsProvidersView.get_provider_data(course_key_string)
return Response({
**config_data,
"features": provider_data["features"],
"providers": {
"active": provider_data["active"],
"available": provider_data["available"],
},
})
def post(self, request, course_key_string: str, **_kwargs) -> Response:
"""
Handle HTTP/POST requests
"""
config_data = self.update_configuration_data(request, course_key_string)
provider_data = DiscussionsProvidersView.get_provider_data(course_key_string)
return Response(
{
**config_data,
"features": provider_data["features"],
"providers": {
"active": provider_data["active"],
"available": provider_data["available"],
},
}
)