Skip to content

Commit

Permalink
Take both a list of strings and comma separated string in get_roles
Browse files Browse the repository at this point in the history
LTI1.1 provides a comma separated list of roles while LTI1.3 provides a
list.

The launch LTIParams provides an abstraction for this, always returning
the LTI1.1 version.

To allow using get_roles outside a launch context without having to
needlessly convert between the two formats allow the method to take
roles in both formats.
  • Loading branch information
marcospri committed Aug 26, 2024
1 parent 92ce837 commit 7ad2a2a
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 6 deletions.
10 changes: 6 additions & 4 deletions lms/services/lti_role_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,17 @@ class LTIRoleService:
def __init__(self, db_session: Session):
self._db = db_session

def get_roles(self, role_description: str) -> list[LTIRole]:
def get_roles(self, role_description: str | list[str]) -> list[LTIRole]:
"""
Get a list of role objects for the provided strings.
:param role_description: A comma delimited set of role strings
:param role_description: A comma delimited set of role strings for LTI1.1 or a list of string for LTI1.3
"""
role_strings = [role.strip() for role in role_description.split(",")]
if isinstance(role_description, str):
role_strings = [role.strip() for role in role_description.split(",")]
else:
role_strings = role_description

# Pylint is confused about the `in_` for some reason
roles = self._db.query(LTIRole).filter(LTIRole.value.in_(role_strings)).all()

for role in roles:
Expand Down
7 changes: 5 additions & 2 deletions tests/unit/lms/services/lti_role_service_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@


class TestLTIRoleService:
def test_get_roles(self, svc, existing_roles):
@pytest.mark.parametrize("roles_as_string", [True, False])
def test_get_roles(self, svc, existing_roles, roles_as_string):
existing_role_strings = [role.value for role in existing_roles]
new_roles = [
"http://purl.imsglobal.org/vocab/lis/v2/system/person#SysSupport",
Expand All @@ -22,7 +23,9 @@ def test_get_roles(self, svc, existing_roles):
role_descriptions.append(existing_roles[0].value)
role_descriptions.extend(new_roles)

roles = svc.get_roles(", ".join(role_descriptions))
if roles_as_string:
role_descriptions = ", ".join(role_descriptions)
roles = svc.get_roles(role_descriptions)

expected_new_roles = [
Any.instance_of(LTIRole).with_attrs({"value": value}) for value in new_roles
Expand Down

0 comments on commit 7ad2a2a

Please sign in to comment.