From 3ca2b4f69f1e5a45adc34f3b621c6d4f20b45706 Mon Sep 17 00:00:00 2001 From: Marcos Prieto Date: Thu, 19 Dec 2024 11:18:43 +0100 Subject: [PATCH] Fix issue while trying to fetch multiple groups rosters --- lms/views/dashboard/api/user.py | 12 ++++++---- lms/views/dashboard/pagination.py | 4 ++-- .../unit/lms/views/dashboard/api/user_test.py | 24 ++++++++----------- 3 files changed, 19 insertions(+), 21 deletions(-) diff --git a/lms/views/dashboard/api/user.py b/lms/views/dashboard/api/user.py index 79edbbcd1b..67ea99552f 100644 --- a/lms/views/dashboard/api/user.py +++ b/lms/views/dashboard/api/user.py @@ -3,7 +3,7 @@ from marshmallow import fields, validate from pyramid.view import view_config -from sqlalchemy import Select +from sqlalchemy import Select, true from lms.js_config_types import ( AnnotationMetrics, @@ -13,7 +13,7 @@ AutoGradingGrade, RosterEntry, ) -from lms.models import Assignment, RoleScope, RoleType, User +from lms.models import Assignment, LMSUser, RoleScope, RoleType, User from lms.security import Permissions from lms.services import UserService from lms.services.auto_grading import AutoGradingService @@ -210,7 +210,7 @@ def _students_query( assignment_ids: list[int], segment_authority_provided_ids: list[str], h_userids: list[str] | None = None, - ) -> tuple[datetime | None, Select]: + ) -> tuple[datetime | None, Select[tuple[LMSUser | User, bool]]]: course_ids = self.request.parsed_params.get("course_ids") # Single assigment fetch if ( @@ -254,7 +254,8 @@ def _students_query( if self.request.user else None, admin_organization_ids=[org.id for org in admin_organizations], - ) + # For launch data we always add the "active" column as true for compatibility with the roster query. + ).add_columns(true()) return None, self.user_service.get_users( role_scope=RoleScope.COURSE, @@ -270,4 +271,5 @@ def _students_query( h_userids=h_userids, # Only users belonging to these segments segment_authority_provided_ids=segment_authority_provided_ids, - ) + # For launch data we always add the "active" column as true for compatibility with the roster query. + ).add_columns(true()) diff --git a/lms/views/dashboard/pagination.py b/lms/views/dashboard/pagination.py index 0cc61a3a87..38325e2708 100644 --- a/lms/views/dashboard/pagination.py +++ b/lms/views/dashboard/pagination.py @@ -1,7 +1,7 @@ import json import logging from types import NoneType -from typing import TypeVar +from typing import Any, TypeVar from urllib.parse import parse_qs, urlencode, urlparse, urlunparse from marshmallow import ValidationError, fields, post_load, validate @@ -39,7 +39,7 @@ def _get_next_url(current_url, cursor_value) -> str: def get_page( - request, items_query: Select[tuple[T]], cursor_columns: list + request, items_query: Select[tuple[T, *tuple[Any]]], cursor_columns: list ) -> tuple[list[T], Pagination]: """Return the first page and pagination metadata from a query.""" diff --git a/tests/unit/lms/views/dashboard/api/user_test.py b/tests/unit/lms/views/dashboard/api/user_test.py index 51ae7dcd50..285dc14714 100644 --- a/tests/unit/lms/views/dashboard/api/user_test.py +++ b/tests/unit/lms/views/dashboard/api/user_test.py @@ -42,7 +42,7 @@ def test_get_students(self, user_service, pyramid_request, views, get_page): ) get_page.assert_called_once_with( pyramid_request, - user_service.get_users.return_value, + user_service.get_users.return_value.add_columns.return_value, [User.display_name, User.id], ) assert response == { @@ -92,21 +92,17 @@ def test_students_metrics( pyramid_request.parsed_params["segment_authority_provided_ids"] = [ g.authority_provided_id for g in segments ] - user_service.get_users.return_value = ( - select(User) - .where( - User.id.in_( - [ - u.id - for u in [ - student, - student_no_annos, - student_no_annos_no_name, - ] + user_service.get_users.return_value = select(User).where( + User.id.in_( + [ + u.id + for u in [ + student, + student_no_annos, + student_no_annos_no_name, ] - ) + ] ) - .add_columns(True) ) else: db_session.flush()