hypothesis / lms

LTI app for integrating with learning management systems
BSD 2-Clause "Simplified" License
46 stars 14 forks source link

Refactor get_user and split it into multiple, single purpose, methods #6862

Closed marcospri closed 2 days ago

marcospri commented 3 days ago

Split the get_user method into three different cases:

For these we use one simpler query that uses the new LMSUser/LMSCourse tables. This queries should be faster unless we are missing some indices.

Note that we still keep the old get_users, this will handle cases for which we have multiple assignments / course and/or segments filters. Those cases are less important for the roster features so will leave them for a follow up refactor.


This PR also uncludes small tweaks found along the way:

Don't call the H api if there's no groups to request. This might happen (at least locally), if you launch as an instructor, open that dashboard and select the segment filter for any section before anybody else launches into those sections.

Sort segments by name, see https://github.com/hypothesis/lms/pull/6862/commits/4ce6f41192f32c55930d4fd9a7b5eaf9e6030347

Testing

I've launched the following as: eng+canvasstudent@hypothes.is

https://hypothesis.instructure.com/courses/125/assignments/873 https://hypothesis.instructure.com/courses/125/assignments/1833 https://hypothesis.instructure.com/courses/319/assignments/3336 https://hypothesis.instructure.com/courses/319/assignments/3347

And the same as an instructor, then opened the dashboard: