Skip to content

Commit

Permalink
Allow None values in non required LTI fields.
Browse files Browse the repository at this point in the history
We have seen at least one school sending `null` here which caused the
validation to fail for a field that was not required at all.
  • Loading branch information
marcospri committed Feb 23, 2024
1 parent 01896a1 commit b555bf7
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 4 deletions.
8 changes: 4 additions & 4 deletions lms/validation/_lti_launch_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ class Meta:
user_id = fields.Str(required=True)
roles = fields.Str(required=True)
tool_consumer_instance_guid = fields.Str(required=True)
lis_person_name_given = fields.Str(load_default="")
lis_person_name_family = fields.Str(load_default="")
lis_person_name_full = fields.Str(load_default="")
lis_person_contact_email_primary = fields.Str(load_default="")
lis_person_name_given = fields.Str(load_default="", allow_none=True)
lis_person_name_family = fields.Str(load_default="", allow_none=True)
lis_person_name_full = fields.Str(load_default="", allow_none=True)
lis_person_contact_email_primary = fields.Str(load_default="", allow_none=True)

@pre_load
def _decode_jwt(self, data, **_kwargs):
Expand Down
16 changes: 16 additions & 0 deletions tests/unit/lms/validation/_lti_launch_params_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,22 @@ def test_it_raises_LTIToolRedirect_if_a_reportable_field_is_missing(
== f"http://example.com?lti_msg=Field+%27{required_param}%27%3A+Missing+data+for+required+field."
)

@pytest.mark.parametrize(
"required_param",
[
"lis_person_name_given",
"lis_person_name_family",
"lis_person_name_full",
"lis_person_contact_email_primary",
],
)
def test_it_allow_none_values(self, pyramid_request, required_param):
pyramid_request.params[required_param] = None

schema = BasicLTILaunchSchema(pyramid_request)

schema.parse()

@pytest.mark.parametrize(
"required_param",
[
Expand Down

0 comments on commit b555bf7

Please sign in to comment.