From 33e499ea049fb8b1c7a4f3bf130e0fcdcf0d2170 Mon Sep 17 00:00:00 2001 From: stvn Date: Thu, 15 Apr 2021 18:40:52 -0700 Subject: [PATCH] fix: Update discussions lti_configuration during provider_type change Else, we risk updating it when we don't intend to, eg: - changing from an LTI-backed provider (Piazza) to a non-LTI-backed provider (legacy) - the settings are no longer relevant - changing from a non-LTI-backed provider (legacy) to an LTI-backed provider (Piazza) - the settings _are_ now relevant --- openedx/core/djangoapps/discussions/views.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/openedx/core/djangoapps/discussions/views.py b/openedx/core/djangoapps/discussions/views.py index 1a46104923..59fa001d6c 100644 --- a/openedx/core/djangoapps/discussions/views.py +++ b/openedx/core/djangoapps/discussions/views.py @@ -150,6 +150,8 @@ class DiscussionsConfigurationView(APIView): value = validated_data.get(key) if value is not None: setattr(instance, key, value) + # _update_* helpers assume `enabled` and `provider_type` + # have already been set instance = self._update_lti(instance, validated_data) instance.save() return instance @@ -159,15 +161,19 @@ class DiscussionsConfigurationView(APIView): Update LtiConfiguration """ lti_configuration_data = validated_data.get('lti_configuration') - if lti_configuration_data: - instance.lti_configuration = instance.lti_configuration or LtiConfiguration() - lti_configuration = LtiSerializer( - instance.lti_configuration, + supports_lti = instance.supports('lti') + if not supports_lti: + instance.lti_configuration = None + elif lti_configuration_data: + lti_configuration = instance.lti_configuration or LtiConfiguration() + lti_serializer = LtiSerializer( + lti_configuration, data=lti_configuration_data, partial=True, ) - if lti_configuration.is_valid(raise_exception=True): - lti_configuration.save() + if lti_serializer.is_valid(raise_exception=True): + lti_serializer.save() + instance.lti_configuration = lti_configuration return instance # pylint: disable=redefined-builtin