From 792940d467fd5d37b951a278fbfb47057bc52423 Mon Sep 17 00:00:00 2001 From: Ibrahim <ibrahimahmed443@gmail.com> Date: Thu, 24 Mar 2016 17:35:46 +0500 Subject: [PATCH] Fix PHX-254 where custom_parameters field is empty by adding field validation --- lti_consumer/lti_consumer.py | 38 +++++++++++--------- lti_consumer/tests/unit/test_lti_consumer.py | 8 +++++ 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/lti_consumer/lti_consumer.py b/lti_consumer/lti_consumer.py index 5140862..a001286 100644 --- a/lti_consumer/lti_consumer.py +++ b/lti_consumer/lti_consumer.py @@ -64,6 +64,7 @@ from django.utils import timezone from xblock.core import String, Scope, List, XBlock from xblock.fields import Boolean, Float, Integer from xblock.fragment import Fragment +from xblock.validation import ValidationMessage from xblockutils.resources import ResourceLoader from xblockutils.studio_editable import StudioEditableXBlockMixin @@ -73,7 +74,6 @@ from .oauth import log_authorization_header from .lti import LtiConsumer from .outcomes import OutcomeService - log = logging.getLogger(__name__) DOCS_ANCHOR_TAG_OPEN = ( @@ -424,6 +424,11 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): 'ask_to_send_username', 'ask_to_send_email' ) + def validate_field_data(self, validation, data): + if not isinstance(data.custom_parameters, list): + _ = self.runtime.service(self, "i18n").ugettext + validation.add(ValidationMessage(ValidationMessage.ERROR, unicode(_("Custom Parameters must be a list")))) + @property def descriptor(self): """ @@ -570,21 +575,22 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): # parsing custom parameters to dict custom_parameters = {} - for custom_parameter in self.custom_parameters: - try: - param_name, param_value = [p.strip() for p in custom_parameter.split('=', 1)] - except ValueError: - _ = self.runtime.service(self, "i18n").ugettext - msg = _('Could not parse custom parameter: {custom_parameter}. Should be "x=y" string.').format( - custom_parameter="{0!r}".format(custom_parameter) - ) - raise LtiError(msg) - - # LTI specs: 'custom_' should be prepended before each custom parameter, as pointed in link above. - if param_name not in LTI_PARAMETERS: - param_name = 'custom_' + param_name - - custom_parameters[unicode(param_name)] = unicode(param_value) + if isinstance(self.custom_parameters, list): + for custom_parameter in self.custom_parameters: + try: + param_name, param_value = [p.strip() for p in custom_parameter.split('=', 1)] + except ValueError: + _ = self.runtime.service(self, "i18n").ugettext + msg = _('Could not parse custom parameter: {custom_parameter}. Should be "x=y" string.').format( + custom_parameter="{0!r}".format(custom_parameter) + ) + raise LtiError(msg) + + # LTI specs: 'custom_' should be prepended before each custom parameter, as pointed in link above. + if param_name not in LTI_PARAMETERS: + param_name = 'custom_' + param_name + + custom_parameters[unicode(param_name)] = unicode(param_value) return custom_parameters diff --git a/lti_consumer/tests/unit/test_lti_consumer.py b/lti_consumer/tests/unit/test_lti_consumer.py index fa2ebe2..0c91b64 100644 --- a/lti_consumer/tests/unit/test_lti_consumer.py +++ b/lti_consumer/tests/unit/test_lti_consumer.py @@ -52,6 +52,14 @@ class TestProperties(TestLtiConsumerXBlock): """ self.assertEqual(self.xblock.context_id, unicode(self.xblock.course_id)) # pylint: disable=no-member + def test_validate(self): + """ + Test that if custom_parameters is empty string, a validation error is added + """ + self.xblock.custom_parameters = '' + validation = self.xblock.validate() + self.assertFalse(validation.empty) + def test_role(self): """ Test `role` returns the correct LTI role string -- GitLab