From e8163c91619c697c8682048b58a89945b59bbbcb Mon Sep 17 00:00:00 2001 From: stvn Date: Wed, 7 Apr 2021 10:32:04 -0700 Subject: [PATCH 1/7] fix: Use the correct name for serializer to_internal_value Fortunately, this wasn't being used yet. --- openedx/core/djangoapps/discussions/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/discussions/views.py b/openedx/core/djangoapps/discussions/views.py index 9949661464..9fd9b3a4e6 100644 --- a/openedx/core/djangoapps/discussions/views.py +++ b/openedx/core/djangoapps/discussions/views.py @@ -44,7 +44,7 @@ class DiscussionsConfigurationView(APIView): """ raise NotImplementedError - def to_internal_data(self, data): + def to_internal_value(self, data): """ Transform the *incoming* primitive data into a native value. """ From 3f6f6e3667208dafd1794df9009453607959ec74 Mon Sep 17 00:00:00 2001 From: stvn Date: Wed, 7 Apr 2021 10:12:44 -0700 Subject: [PATCH 2/7] refactor: Remove unused helper from discussions API --- openedx/core/djangoapps/discussions/views.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/openedx/core/djangoapps/discussions/views.py b/openedx/core/djangoapps/discussions/views.py index 9fd9b3a4e6..db731327a3 100644 --- a/openedx/core/djangoapps/discussions/views.py +++ b/openedx/core/djangoapps/discussions/views.py @@ -38,12 +38,6 @@ class DiscussionsConfigurationView(APIView): Serialize configuration responses """ - def create(self, validated_data): - """ - Create and save a new instance - """ - raise NotImplementedError - def to_internal_value(self, data): """ Transform the *incoming* primitive data into a native value. From 0cce315539ba9890d2ac3b13c5639f7604f5550e Mon Sep 17 00:00:00 2001 From: stvn Date: Wed, 7 Apr 2021 10:08:56 -0700 Subject: [PATCH 3/7] refactor: Base discussions serializer off of ModelSerializer to make Django operations easier. --- openedx/core/djangoapps/discussions/views.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/openedx/core/djangoapps/discussions/views.py b/openedx/core/djangoapps/discussions/views.py index db731327a3..38a1ff962a 100644 --- a/openedx/core/djangoapps/discussions/views.py +++ b/openedx/core/djangoapps/discussions/views.py @@ -33,10 +33,17 @@ class DiscussionsConfigurationView(APIView): """ permission_classes = (IsStaff,) - class Serializer(serializers.BaseSerializer): + class Serializer(serializers.ModelSerializer): """ Serialize configuration responses """ + class Meta: + model = DiscussionsConfiguration + fields = [ + 'context_key', + 'enabled', + 'provider_type', + ] def to_internal_value(self, data): """ @@ -48,9 +55,8 @@ class DiscussionsConfigurationView(APIView): """ Serialize data into a dictionary, to be used as a response """ - payload = { - 'context_key': str(instance.context_key), - 'enabled': instance.enabled, + payload = super().to_representation(instance) + payload.update({ 'features': { 'discussion-page', 'embedded-course-sections', @@ -67,7 +73,7 @@ class DiscussionsConfigurationView(APIView): for provider in instance.available_providers }, }, - } + }) return payload def update(self, instance, validated_data): From def644b9aa46b4c15cb1042ee895eef653e8814c Mon Sep 17 00:00:00 2001 From: stvn Date: Wed, 7 Apr 2021 10:09:59 -0700 Subject: [PATCH 4/7] feat: Add lti_configuration to discussions API payload --- openedx/core/djangoapps/discussions/views.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/openedx/core/djangoapps/discussions/views.py b/openedx/core/djangoapps/discussions/views.py index 38a1ff962a..416dbf20dc 100644 --- a/openedx/core/djangoapps/discussions/views.py +++ b/openedx/core/djangoapps/discussions/views.py @@ -1,6 +1,7 @@ """ Handle view-logic for the djangoapp """ +from lti_consumer.models import LtiConfiguration from opaque_keys.edx.keys import CourseKey from opaque_keys import InvalidKeyError from rest_framework import serializers @@ -26,6 +27,20 @@ PROVIDER_FEATURE_MAP = { } +class LtiSerializer(serializers.ModelSerializer): + """ + Serialize LtiConfiguration responses + """ + class Meta: + model = LtiConfiguration + fields = [ + 'lti_1p1_client_key', + 'lti_1p1_client_secret', + 'lti_1p1_launch_url', + 'version', + ] + + @view_auth_classes() class DiscussionsConfigurationView(APIView): """ @@ -56,6 +71,7 @@ class DiscussionsConfigurationView(APIView): Serialize data into a dictionary, to be used as a response """ payload = super().to_representation(instance) + lti_configuration = LtiSerializer(instance.lti_configuration) payload.update({ 'features': { 'discussion-page', @@ -63,6 +79,7 @@ class DiscussionsConfigurationView(APIView): 'lti', 'wcag-2.1', }, + 'lti_configuration': lti_configuration.data, 'plugin_configuration': instance.plugin_configuration, 'providers': { 'active': instance.provider_type or None, From 342d922034aaa4f97deffc9223430fe60b202c79 Mon Sep 17 00:00:00 2001 From: stvn Date: Wed, 7 Apr 2021 10:32:51 -0700 Subject: [PATCH 5/7] feat: Implement to_internal_value helper for discussion API --- openedx/core/djangoapps/discussions/views.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/discussions/views.py b/openedx/core/djangoapps/discussions/views.py index 416dbf20dc..cc49238b1c 100644 --- a/openedx/core/djangoapps/discussions/views.py +++ b/openedx/core/djangoapps/discussions/views.py @@ -64,7 +64,14 @@ class DiscussionsConfigurationView(APIView): """ Transform the *incoming* primitive data into a native value. """ - raise NotImplementedError + payload = { + 'context_key': data.get('course_key', ''), + 'enabled': data.get('enabled', False), + 'lti_configuration': data.get('lti_configuration', {}), + 'plugin_configuration': data.get('plugin_configuration', {}), + 'provider_type': data.get('provider_type', ''), + } + return payload def to_representation(self, instance) -> dict: """ From 87e375d6271290d147c3d08fb14d1c8b2427266d Mon Sep 17 00:00:00 2001 From: stvn Date: Wed, 7 Apr 2021 10:14:23 -0700 Subject: [PATCH 6/7] feat: Implement update helper for discussions API --- openedx/core/djangoapps/discussions/views.py | 24 +++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/discussions/views.py b/openedx/core/djangoapps/discussions/views.py index cc49238b1c..9fee97da54 100644 --- a/openedx/core/djangoapps/discussions/views.py +++ b/openedx/core/djangoapps/discussions/views.py @@ -104,7 +104,29 @@ class DiscussionsConfigurationView(APIView): """ Update and save an existing instance """ - raise NotImplementedError + keys = [ + 'enabled', + 'plugin_configuration', + 'provider_type', + ] + for key in keys: + value = validated_data.get(key) + if value is not None: + setattr(instance, key, value) + lti_configuration_data = validated_data.get('lti_configuration') + if lti_configuration_data: + lti_key = lti_configuration_data.get('lti_1p1_client_key') + lti_secret = lti_configuration_data.get('lti_1p1_client_secret') + lti_url = lti_configuration_data.get('lti_1p1_launch_url') + lti_configuration = instance.lti_configuration or LtiConfiguration() + lti_configuration.config_store = LtiConfiguration.CONFIG_ON_DB + lti_configuration.lti_1p1_client_key = lti_key + lti_configuration.lti_1p1_client_secret = lti_secret + lti_configuration.lti_1p1_launch_url = lti_url + lti_configuration.save() + instance.lti_configuration = lti_configuration + instance.save() + return instance # pylint: disable=redefined-builtin def get(self, request, course_key_string, **_kwargs) -> Response: From 6eabae0d9eb5f0a0948853c78f2bb9e7e9c7c897 Mon Sep 17 00:00:00 2001 From: stvn Date: Wed, 7 Apr 2021 10:14:43 -0700 Subject: [PATCH 7/7] feat: Implement POST endpoint for discussions API --- openedx/core/djangoapps/discussions/views.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/openedx/core/djangoapps/discussions/views.py b/openedx/core/djangoapps/discussions/views.py index 9fee97da54..e47662f4d3 100644 --- a/openedx/core/djangoapps/discussions/views.py +++ b/openedx/core/djangoapps/discussions/views.py @@ -138,6 +138,19 @@ class DiscussionsConfigurationView(APIView): serializer = self.Serializer(configuration) return Response(serializer.data) + def post(self, request, course_key_string, **_kwargs) -> Response: + """ + Handle HTTP/POST requests + + TODO: Should we cleanup orphaned LTI config when swapping to cs_comments_service? + """ + course_key = self._validate_course_key(course_key_string) + configuration = DiscussionsConfiguration.get(course_key) + serializer = self.Serializer(configuration, data=request.data, partial=True) + if serializer.is_valid(raise_exception=True): + serializer.save() + return Response(serializer.data) + def _validate_course_key(self, course_key_string: str) -> CourseKey: """ Validate and parse a course_key string, if supported