From 0acd7e23b58350d6120959ea10b6cbbed6b5bf15 Mon Sep 17 00:00:00 2001 From: Marcos Prieto Date: Wed, 4 Dec 2024 12:53:22 +0100 Subject: [PATCH] Fetch canvas groups rosters using the names and roles API Canvas provides an extension to the names and roles API that allows to fetch the members of a group. See: https://canvas.instructure.com/doc/api/names_and_role.html#method.lti/ims/names_and_roles.group_index --- lms/services/lti_names_roles.py | 2 +- lms/services/roster.py | 74 ++++++++++++++++++++++++++ tests/factories/__init__.py | 2 +- tests/factories/roster.py | 3 ++ tests/unit/lms/services/roster_test.py | 62 ++++++++++++++++++++- 5 files changed, 140 insertions(+), 3 deletions(-) diff --git a/lms/services/lti_names_roles.py b/lms/services/lti_names_roles.py index 93701c661e..1624915dfc 100644 --- a/lms/services/lti_names_roles.py +++ b/lms/services/lti_names_roles.py @@ -44,7 +44,7 @@ def get_context_memberships( The course is defined by the service URL which will obtain from a LTI launch parameter and is always linked to an specific context. - Optically, using the same service_url the API allows to get the roster of an assignment identified by `resource_link_id`. + Optionally, using the same service_url the API allows to get the roster of an assignment identified by `resource_link_id`. """ query = {} if resource_link_id: diff --git a/lms/services/roster.py b/lms/services/roster.py index 71164cb6e2..e57ef2aa41 100644 --- a/lms/services/roster.py +++ b/lms/services/roster.py @@ -9,6 +9,8 @@ CourseRoster, LMSCourse, LMSCourseApplicationInstance, + LMSSegment, + LMSSegmentRoster, LMSUser, LTIRegistration, LTIRole, @@ -246,6 +248,78 @@ def fetch_assignment_roster(self, assignment: Assignment) -> None: update_columns=["active", "updated"], ) + def fetch_canvas_group_roster(self, canvas_group: LMSSegment) -> None: + """Fetch the roster information for an assignment from the LMS.""" + assert canvas_group.type == "canvas_group" + + lms_course = canvas_group.lms_course + assert ( + lms_course.lti_context_memberships_url + ), "Trying fetch roster for course without service URL." + + application_instance = self._db.scalars( + select(ApplicationInstance) + .join(LMSCourseApplicationInstance) + .where( + LMSCourseApplicationInstance.lms_course_id == lms_course.id, + ApplicationInstance.lti_registration_id.is_not(None), + ) + .order_by(ApplicationInstance.updated.desc()) + ).first() + + roster = self._lti_names_roles_service.get_context_memberships( + application_instance.lti_registration, + # We won't use the names and roles endpoint for groups, we need to pass a URL from the Canvas extension to the API. + # https://canvas.instructure.com/doc/api/names_and_role.html#method.lti/ims/names_and_roles.group_index + f"https://{application_instance.lms_host()}/api/lti/groups/{canvas_group.lms_id}/names_and_roles", + ) + + # Insert any users we might be missing in the DB + lms_users_by_lti_user_id = { + u.lti_user_id: u + for u in self._get_roster_users( + roster, lms_course.tool_consumer_instance_guid + ) + } + # Also insert any roles we might be missing + lti_roles_by_value: dict[str, LTIRole] = { + r.value: r for r in self._get_roster_roles(roster) + } + + # Make sure any new rows have IDs + self._db.flush() + + roster_upsert_elements = [] + + for member in roster: + lti_user_id = member.get("lti11_legacy_user_id") or member["user_id"] + # Now, for every user + role, insert a row in the roster table + for role in member["roles"]: + roster_upsert_elements.append( + { + "lms_segment_id": canvas_group.id, + "lms_user_id": lms_users_by_lti_user_id[lti_user_id].id, + "lti_role_id": lti_roles_by_value[role].id, + "active": member["status"] == "Active", + } + ) + # We'll first mark everyone as non-Active. + # We keep a record of who belonged to a course even if they are no longer present. + self._db.execute( + update(LMSSegmentRoster) + .where(LMSSegmentRoster.lms_segment_id == canvas_group.id) + .values(active=False) + ) + + # Insert and update roster rows. + bulk_upsert( + self._db, + LMSSegmentRoster, + values=roster_upsert_elements, + index_elements=["lms_segment_id", "lms_user_id", "lti_role_id"], + update_columns=["active", "updated"], + ) + def _get_roster_users(self, roster, tool_consumer_instance_guid): values = [] for member in roster: diff --git a/tests/factories/__init__.py b/tests/factories/__init__.py index 64cfb38cbf..486e1eabb3 100644 --- a/tests/factories/__init__.py +++ b/tests/factories/__init__.py @@ -46,7 +46,7 @@ from tests.factories.oauth2_token import OAuth2Token from tests.factories.organization import Organization from tests.factories.organization_usage import OrganizationUsageReport -from tests.factories.roster import AssignmentRoster, CourseRoster +from tests.factories.roster import AssignmentRoster, CourseRoster, LMSSegmentRoster from tests.factories.rsa_key import RSAKey from tests.factories.task_done import TaskDone from tests.factories.user import User diff --git a/tests/factories/roster.py b/tests/factories/roster.py index 7e9d414a60..27d8a2145f 100644 --- a/tests/factories/roster.py +++ b/tests/factories/roster.py @@ -7,3 +7,6 @@ AssignmentRoster = make_factory( models.AssignmentRoster, FACTORY_CLASS=SQLAlchemyModelFactory ) +LMSSegmentRoster = make_factory( + models.LMSSegmentRoster, FACTORY_CLASS=SQLAlchemyModelFactory +) diff --git a/tests/unit/lms/services/roster_test.py b/tests/unit/lms/services/roster_test.py index ee54a6d0e2..1aa1fe20a0 100644 --- a/tests/unit/lms/services/roster_test.py +++ b/tests/unit/lms/services/roster_test.py @@ -4,7 +4,7 @@ from h_matchers import Any from sqlalchemy import select -from lms.models import AssignmentRoster, CourseRoster +from lms.models import AssignmentRoster, CourseRoster, LMSSegmentRoster from lms.services.exceptions import ExternalRequestError from lms.services.roster import RosterService, factory from tests import factories @@ -278,6 +278,66 @@ def test_fetch_assignment_roster_raises_external_request_error( with pytest.raises(ExternalRequestError): svc.fetch_assignment_roster(assignment) + def test_fetch_canvas_group_roster( + self, + svc, + lti_names_roles_service, + lti_v13_application_instance, + db_session, + names_and_roles_roster_response, + lti_role_service, + lms_course, + ): + canvas_group = factories.LMSSegment(type="canvas_group", lms_course=lms_course) + # Active user not returned by the roster, should be marked inactive after fetch the roster + factories.LMSSegmentRoster( + lms_segment=canvas_group, + lms_user=factories.LMSUser(lti_user_id="EXISTING USER"), + lti_role=factories.LTIRole(), + active=True, + ) + db_session.flush() + lti_names_roles_service.get_context_memberships.return_value = ( + names_and_roles_roster_response + ) + lti_role_service.get_roles.return_value = [ + factories.LTIRole(value="ROLE1"), + factories.LTIRole(value="ROLE2"), + ] + + svc.fetch_canvas_group_roster(canvas_group) + + lti_names_roles_service.get_context_memberships.assert_called_once_with( + lti_v13_application_instance.lti_registration, + f"https://{lti_v13_application_instance.lms_host()}/api/lti/groups/{canvas_group.lms_id}/names_and_roles", + ) + lti_role_service.get_roles.assert_called_once_with( + Any.list.containing(["ROLE2", "ROLE1"]) + ) + + roster = db_session.scalars( + select(LMSSegmentRoster) + .order_by(LMSSegmentRoster.lms_user_id) + .where(LMSSegmentRoster.lms_segment_id == canvas_group.id) + ).all() + + assert len(roster) == 4 + assert roster[0].lms_segment_id == canvas_group.id + assert roster[0].lms_user.lti_user_id == "EXISTING USER" + assert not roster[0].active + + assert roster[1].lms_segment_id == canvas_group.id + assert roster[1].lms_user.lti_user_id == "USER_ID" + assert roster[1].active + + assert roster[2].lms_segment_id == canvas_group.id + assert roster[2].lms_user.lti_user_id == "USER_ID" + assert roster[2].active + + assert roster[3].lms_segment_id == canvas_group.id + assert roster[3].lms_user.lti_user_id == "USER_ID_INACTIVE" + assert not roster[3].active + @pytest.fixture def lms_course(self, lti_v13_application_instance): lms_course = factories.LMSCourse(lti_context_memberships_url="SERVICE_URL")