-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expose course rosters on the dashboard #6925
base: main
Are you sure you want to change the base?
Conversation
8cc6281
to
72e23fa
Compare
Although we don't expose a list of students with metrics for a full course we do list students of a course on the dropdowns. This commit uses the roster data for the dropdown when the feature flag is enabled and the data is aviailable.
72e23fa
to
c8d2b81
Compare
@@ -210,7 +211,40 @@ def get_assignment_roster( | |||
assignment_id=assignment.id, | |||
h_userids=h_userids, | |||
# For launch data we always add the "active" column as true for compatibility with the roster query. | |||
).add_columns(True) | |||
).add_columns(true()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
true() is the right type for this.
.join(LTIRole) | ||
.where( | ||
CourseRoster.lms_course_id == lms_course.id, | ||
CourseRoster.active.is_(True), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We now return all rows, not only active ones
lms/services/roster.py
Outdated
@@ -46,34 +46,45 @@ def get_course_roster( | |||
lms_course: LMSCourse, | |||
role_scope: RoleScope | None = None, | |||
role_type: RoleType | None = None, | |||
) -> Select[tuple[LMSUser]]: | |||
h_userids: list[str] | None = None, | |||
) -> Select[tuple[LMSUser, bool]]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We return a tuple of (LMSUser, active)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good in general, and it works as described.
Segment rosters are used a bit differently than assignments ones. We allow filtering by multiple rosters and we'll display a list of those students with the metrics. For assignments we only allow to select one assignment at the time. This adds a bit of extra complexity as we need to query the roster as a combination of rosters
67a9e6f
to
4541d98
Compare
Although we don't expose a list of students with metrics for a full course we do list students of a course on the dropdowns.
This commit uses the roster data for the dropdown when the feature flag is enabled and the data is available.
Testing
http://localhost:8001/admin/instances/102/settings
https://hypothesis.instructure.com/courses/319/assignments/3308
make shell
Open the dashboard for the assignment
Navigate un level up, to the course, open the student drop down. It includes all students in the course, not only the ones that have launched.