diff --git a/app/discord_bot.py b/app/discord_bot.py index f901281..b3102b3 100644 --- a/app/discord_bot.py +++ b/app/discord_bot.py @@ -683,9 +683,10 @@ class TeamTryoutsBot(commands.Bot): reference_id: ID of the MatchParticipant or TryoutRegistration record. """ # Look up the DB user to get their Discord user ID + from app.extensions import db from app.models import User as DBUser - db_user = DBUser.query.get(user_id) + db_user = db.session.get(DBUser, user_id) if not db_user: logger.warning(f"DB user {user_id} not found for schedule notification") return None @@ -754,7 +755,7 @@ class TeamTryoutsBot(commands.Bot): from app.models import OneOnOneRequest try: - request = OneOnOneRequest.query.get(request_id) + request = db.session.get(OneOnOneRequest, request_id) if not request: # The row is gone; no reaction on this message can ever mean # anything again. Keeping the mapping is what PENDING_MAX_AGE_DAYS @@ -825,7 +826,7 @@ class TeamTryoutsBot(commands.Bot): from app.models import OneOnOneRequest try: - request = OneOnOneRequest.query.get(request_id) + request = db.session.get(OneOnOneRequest, request_id) if not request: logger.info( 'One on One request %s no longer exists; its pending message was dropped.', @@ -918,12 +919,13 @@ class TeamTryoutsBot(commands.Bot): Returns: tuple: (row, player_id) — either may be None. """ + from app.extensions import db from app.models import MatchParticipant, TryoutRegistration if event_type == 'match': - row = MatchParticipant.query.get(reference_id) + row = db.session.get(MatchParticipant, reference_id) elif event_type == 'tryout': - row = TryoutRegistration.query.get(reference_id) + row = db.session.get(TryoutRegistration, reference_id) else: row = None return row, getattr(row, 'player_id', None) @@ -937,11 +939,12 @@ class TeamTryoutsBot(commands.Bot): attendance handlers did not (OPS-009) — same message shape, same threat, one of them checked. The asymmetry was the bug. """ + from app.extensions import db from app.models import User if not player_id: return False - owner = User.query.get(player_id) + owner = db.session.get(User, player_id) return bool(owner and owner.discord_user_id == str(reacting_user.id)) async def handle_attendance_confirm(self, player, message_id, reference_id, channel): diff --git a/app/models/contract.py b/app/models/contract.py index 889a9ac..c18ea94 100644 --- a/app/models/contract.py +++ b/app/models/contract.py @@ -57,7 +57,7 @@ class Contract(db.Model): if isinstance(user, Admin): return True if isinstance(user, Manager): - player = User.query.get(self.player_id) + player = db.session.get(User, self.player_id) if player and player.get_org_teams(): return True if isinstance(user, Coach): diff --git a/app/routes/evaluations.py b/app/routes/evaluations.py index a6aa2cc..e9d0c20 100644 --- a/app/routes/evaluations.py +++ b/app/routes/evaluations.py @@ -27,6 +27,14 @@ from app.validators import EvaluationSchema evaluations_bp = Blueprint('evaluations', __name__, url_prefix='/evaluations') +def _users_by_id(user_ids): + """Load a set of users once for aggregate/list views.""" + wanted = {user_id for user_id in user_ids if user_id} + if not wanted: + return {} + return {user.id: user for user in User.query.filter(User.id.in_(wanted)).all()} + + @evaluations_bp.route('') @login_required def list_evaluations(): @@ -85,9 +93,10 @@ def list_evaluations(): .group_by(Evaluation.player_id) .all() ) + players_by_id = _users_by_id(row.player_id for row in avg_scores) player_scores = {} for row in avg_scores: - p = User.query.get(row.player_id) + p = players_by_id.get(row.player_id) if p: player_scores[p.id] = { 'player': p, @@ -126,7 +135,7 @@ def evaluate_player(tryout_id, player_id): flash(_('You do not have permission to evaluate players.'), 'danger') return redirect(url_for('main.dashboard')) - tryout = Tryout.query.get_or_404(tryout_id) + tryout = db.get_or_404(Tryout, tryout_id) if not current_user.can_manage_this_tryout(tryout): flash(_('You do not have permission to evaluate players in this tryout.'), 'danger') return redirect(url_for('tryouts.list_tryouts')) @@ -142,7 +151,7 @@ def evaluate_player(tryout_id, player_id): flash(_('Player is not registered for this tryout.'), 'danger') return redirect(url_for('tryouts.view_tryout', tryout_id=tryout_id)) - player = User.query.get_or_404(player_id) + player = db.get_or_404(User, player_id) if not isinstance(player, Player): flash(_('Can only evaluate players.'), 'danger') return redirect(url_for('tryouts.view_tryout', tryout_id=tryout_id)) @@ -160,8 +169,10 @@ def evaluate_player(tryout_id, player_id): tryout_id=tryout_id, player_id=player_id, ).all() + evaluators_by_id = _users_by_id(e.evaluator_id for e in all_evaluations) evaluators = [ - {'evaluator': User.query.get(e.evaluator_id), 'eval': e} for e in all_evaluations + {'evaluator': evaluators_by_id.get(e.evaluator_id), 'eval': e} + for e in all_evaluations ] return render_template( @@ -210,21 +221,27 @@ def players_to_evaluate(tryout_id): flash(_('Permission denied.'), 'danger') return redirect(url_for('main.dashboard')) - tryout = Tryout.query.get_or_404(tryout_id) + tryout = db.get_or_404(Tryout, tryout_id) if not current_user.can_manage_this_tryout(tryout): flash(_('You do not have permission to evaluate players in this tryout.'), 'danger') return redirect(url_for('tryouts.list_tryouts')) registrations = TryoutRegistration.query.filter_by(tryout_id=tryout_id).all() - players = [] - for reg in registrations: - p = User.query.get(reg.player_id) - if p and isinstance(p, Player): - existing = Evaluation.query.filter_by( - tryout_id=tryout_id, - player_id=p.id, - evaluator_id=current_user.id, - ).first() - players.append({'player': p, 'evaluated': existing is not None, 'registration': reg}) + players_by_id = _users_by_id(reg.player_id for reg in registrations) + evaluated_player_ids = { + player_id + for (player_id,) in db.session.query(Evaluation.player_id) + .filter_by(tryout_id=tryout_id, evaluator_id=current_user.id) + .all() + } + players = [ + { + 'player': player, + 'evaluated': player.id in evaluated_player_ids, + 'registration': registration, + } + for registration in registrations + if (player := players_by_id.get(registration.player_id)) and isinstance(player, Player) + ] return render_template('pages/players_to_evaluate.html', tryout=tryout, players=players) diff --git a/app/routes/main.py b/app/routes/main.py index 2691b3d..5696c29 100644 --- a/app/routes/main.py +++ b/app/routes/main.py @@ -223,20 +223,18 @@ def dashboard(): elif isinstance(user, Scout): stats['total_players'] = User.query.filter_by(role='player').count() stats['total_evaluations'] = Evaluation.query.count() - stats['avg_scores'] = ( + top_rows = ( db.session.query( - Evaluation.player_id, + User, func.avg(Evaluation.overall_score).label('avg_score'), ) - .group_by(Evaluation.player_id) - .order_by(func.avg(Evaluation.overall_score).desc()) + .join(Evaluation, Evaluation.player_id == User.id) + .filter(User.role == 'player') + .group_by(User.id) + .order_by(func.avg(Evaluation.overall_score).desc(), User.id) .limit(5) .all() ) - stats['top_players'] = [] - for row in stats['avg_scores']: - p = User.query.get(row.player_id) - if p: - stats['top_players'].append((p, round(row.avg_score, 1))) + stats['top_players'] = [(player, round(avg_score, 1)) for player, avg_score in top_rows] return render_template('pages/dashboard.html', user=user, stats=stats) diff --git a/app/routes/matches.py b/app/routes/matches.py index d7cfa43..6f6f4ed 100644 --- a/app/routes/matches.py +++ b/app/routes/matches.py @@ -289,7 +289,7 @@ def api_events(): @login_required def api_events_for_tryout(tryout_id): """API endpoint returning calendar events for a specific tryout.""" - tryout = Tryout.query.get_or_404(tryout_id) + tryout = db.get_or_404(Tryout, tryout_id) can_view = current_user.can_manage_this_tryout(tryout) is_registered = False @@ -378,7 +378,7 @@ def api_events_for_tryout(tryout_id): @login_required def create_match(tryout_id): """Create a new match / scrimmage within a tryout.""" - tryout = Tryout.query.get_or_404(tryout_id) + tryout = db.get_or_404(Tryout, tryout_id) if not current_user.can_manage_this_tryout(tryout): flash(_('You do not have permission to schedule matches for this tryout.'), 'danger') return redirect(url_for('tryouts.view_tryout', tryout_id=tryout_id)) @@ -452,7 +452,7 @@ def create_match(tryout_id): @login_required def edit_match(match_id): """Edit an existing match.""" - match = Match.query.get_or_404(match_id) + match = db.get_or_404(Match, match_id) tryout = match.tryout if not current_user.can_manage_this_tryout(tryout): @@ -582,7 +582,7 @@ def api_manageable_tryouts(): @login_required def delete_match(match_id): """Delete a match.""" - match = Match.query.get_or_404(match_id) + match = db.get_or_404(Match, match_id) tryout = match.tryout if not current_user.can_manage_this_tryout(tryout): flash(_('You do not have permission to delete this match.'), 'danger') @@ -669,10 +669,10 @@ def api_available_players(date, time): @login_required def toggle_presence(match_id, participant_id): """Toggle attendance_confirmed for a match participant.""" - match = Match.query.get_or_404(match_id) + match = db.get_or_404(Match, match_id) tryout = match.tryout - participant = MatchParticipant.query.get_or_404(participant_id) + participant = db.get_or_404(MatchParticipant, participant_id) if participant.match_id != match_id: return jsonify({'error': 'Participant does not belong to this match'}), 400 diff --git a/app/routes/team_matches.py b/app/routes/team_matches.py index bf45954..7671c94 100644 --- a/app/routes/team_matches.py +++ b/app/routes/team_matches.py @@ -108,7 +108,7 @@ def list_matches(): @login_required def create_match(team_id): """Create a new regular-season team match.""" - team = OrgTeam.query.get_or_404(team_id) + team = db.get_or_404(OrgTeam, team_id) if not can_manage_team_match(team): flash(_('You do not have permission to schedule matches for this team.'), 'danger') return redirect(url_for('team_matches.list_matches')) @@ -213,7 +213,7 @@ def create_match(team_id): @login_required def edit_match(match_id): """Edit an existing team match.""" - team_match = TeamMatch.query.get_or_404(match_id) + team_match = db.get_or_404(TeamMatch, match_id) team = team_match.org_team if not can_manage_team_match(team): @@ -259,7 +259,7 @@ def edit_match(match_id): @login_required def delete_match(match_id): """Delete a team match.""" - team_match = TeamMatch.query.get_or_404(match_id) + team_match = db.get_or_404(TeamMatch, match_id) team = team_match.org_team if not can_manage_team_match(team): flash(_('You do not have permission to delete this match.'), 'danger') @@ -293,10 +293,10 @@ def api_manageable_teams(): @login_required def toggle_presence(match_id, participant_id): """Toggle is_confirmed for a team match participant.""" - team_match = TeamMatch.query.get_or_404(match_id) + team_match = db.get_or_404(TeamMatch, match_id) team = team_match.org_team - participant = TeamMatchParticipant.query.get_or_404(participant_id) + participant = db.get_or_404(TeamMatchParticipant, participant_id) if participant.team_match_id != match_id: return jsonify({'error': 'Participant does not belong to this match'}), 400 diff --git a/app/routes/teams.py b/app/routes/teams.py index 898a22e..9698d2a 100644 --- a/app/routes/teams.py +++ b/app/routes/teams.py @@ -223,7 +223,7 @@ def create_team(): @login_required def edit_team(team_id): """Edit an existing organization team.""" - team = OrgTeam.query.get_or_404(team_id) + team = db.get_or_404(OrgTeam, team_id) if not current_user.can_manage_this_org_team(team): flash(_('You do not have permission to edit this team.'), 'danger') return redirect(url_for('teams.list_teams')) @@ -294,7 +294,7 @@ def delete_team(team_id): day `Manager.can_manage_this_org_team` is narrowed — which it should be — deletion narrows with it instead of staying the one way in. """ - team = OrgTeam.query.get_or_404(team_id) + team = db.get_or_404(OrgTeam, team_id) if not (current_user.can_manage_teams() and current_user.can_manage_this_org_team(team)): flash(_('You do not have permission to delete teams.'), 'danger') @@ -336,7 +336,7 @@ def delete_team(team_id): @login_required def add_coach(team_id): """Add a coach to an organization team.""" - team = OrgTeam.query.get_or_404(team_id) + team = db.get_or_404(OrgTeam, team_id) if not current_user.can_manage_this_org_team(team): flash(_('Permission denied.'), 'danger') return redirect(url_for('teams.list_teams')) @@ -379,7 +379,7 @@ def add_coach(team_id): @login_required def add_manager(team_id): """Add a manager to an organization team.""" - team = OrgTeam.query.get_or_404(team_id) + team = db.get_or_404(OrgTeam, team_id) if not current_user.can_manage_this_org_team(team): flash(_('Permission denied.'), 'danger') return redirect(url_for('teams.list_teams')) @@ -422,7 +422,7 @@ def add_manager(team_id): @login_required def remove_coach(team_id): """Remove a coach from an organization team.""" - team = OrgTeam.query.get_or_404(team_id) + team = db.get_or_404(OrgTeam, team_id) if not current_user.can_manage_this_org_team(team): flash(_('Permission denied.'), 'danger') return redirect(url_for('teams.list_teams')) @@ -450,7 +450,7 @@ def remove_coach(team_id): @login_required def remove_manager(team_id): """Remove a manager from an organization team.""" - team = OrgTeam.query.get_or_404(team_id) + team = db.get_or_404(OrgTeam, team_id) if not current_user.can_manage_this_org_team(team): flash(_('Permission denied.'), 'danger') return redirect(url_for('teams.list_teams')) @@ -478,7 +478,7 @@ def remove_manager(team_id): @login_required def add_player(team_id): """Add a player to an organization team.""" - team = OrgTeam.query.get_or_404(team_id) + team = db.get_or_404(OrgTeam, team_id) if not current_user.can_manage_this_org_team(team): flash(_('Permission denied.'), 'danger') return redirect(url_for('teams.list_teams')) @@ -515,12 +515,12 @@ def add_player(team_id): @login_required def remove_player(team_id, player_id): """Remove a player from an organization team.""" - team = OrgTeam.query.get_or_404(team_id) + team = db.get_or_404(OrgTeam, team_id) if not current_user.can_manage_this_org_team(team): flash(_('Permission denied.'), 'danger') return redirect(url_for('teams.list_teams')) - player = User.query.get_or_404(player_id) + player = db.get_or_404(User, player_id) tp = TeamPlayer.query.filter_by(player_id=player_id, org_team_id=team_id).first() if not tp: flash( @@ -543,7 +543,7 @@ def remove_player(team_id, player_id): @login_required def toggle_player_status(team_id, player_id): """Toggle a player's status between starter and substitute.""" - team = OrgTeam.query.get_or_404(team_id) + team = db.get_or_404(OrgTeam, team_id) if not current_user.can_manage_this_org_team(team): return jsonify({'error': 'Permission denied'}), 403 @@ -567,7 +567,7 @@ def toggle_player_status(team_id, player_id): @login_required def add_team_note(team_id): """Add a team improvement note (coaches only).""" - team = OrgTeam.query.get_or_404(team_id) + team = db.get_or_404(OrgTeam, team_id) if not current_user.can_manage_this_org_team(team): flash(_('You do not have permission to add notes to this team.'), 'danger') return redirect(url_for('teams.list_teams')) @@ -589,12 +589,12 @@ def add_team_note(team_id): @login_required def add_player_note(team_id, player_id): """Add a personal note for a player (coaches only).""" - team = OrgTeam.query.get_or_404(team_id) + team = db.get_or_404(OrgTeam, team_id) if not current_user.can_manage_this_org_team(team): flash(_('You do not have permission to add notes to this team.'), 'danger') return redirect(url_for('teams.list_teams')) - player = User.query.get_or_404(player_id) + player = db.get_or_404(User, player_id) if not isinstance(player, Player): flash(_('Can only add notes for players.'), 'danger') return redirect(url_for('teams.list_teams')) diff --git a/app/routes/tryouts.py b/app/routes/tryouts.py index 7caadeb..af99be7 100644 --- a/app/routes/tryouts.py +++ b/app/routes/tryouts.py @@ -179,7 +179,7 @@ def create_tryout(): @login_required def edit_tryout(tryout_id): """Edit an existing tryout event. Permission based on can_manage_this_tryout.""" - tryout = Tryout.query.get_or_404(tryout_id) + tryout = db.get_or_404(Tryout, tryout_id) if not current_user.can_manage_this_tryout(tryout): flash(_('You do not have permission to edit this tryout.'), 'danger') @@ -236,7 +236,7 @@ def edit_tryout(tryout_id): @login_required def view_tryout(tryout_id): """View a specific tryout with all details. Permission via polymorphic dispatch.""" - tryout = Tryout.query.get_or_404(tryout_id) + tryout = db.get_or_404(Tryout, tryout_id) can_view = False if isinstance(current_user, Admin): @@ -466,7 +466,7 @@ def register_for_tryout(tryout_id): @login_required def update_status(tryout_id): """Update the status of a tryout.""" - tryout = Tryout.query.get_or_404(tryout_id) + tryout = db.get_or_404(Tryout, tryout_id) if not current_user.can_manage_this_tryout(tryout): flash(_('Permission denied.'), 'danger') return redirect(url_for('tryouts.list_tryouts')) @@ -486,7 +486,7 @@ def update_status(tryout_id): @login_required def update_registration_status(tryout_id, player_id): """Update a registration's attendance status.""" - tryout = Tryout.query.get_or_404(tryout_id) + tryout = db.get_or_404(Tryout, tryout_id) if not current_user.can_manage_this_tryout(tryout): flash(_('Permission denied.'), 'danger') return redirect(url_for('tryouts.list_tryouts')) @@ -557,12 +557,12 @@ def register_player(tryout_id): @login_required def remove_player(tryout_id, player_id): """Remove a registered player from a tryout (cascades to teams/matches).""" - tryout = Tryout.query.get_or_404(tryout_id) + tryout = db.get_or_404(Tryout, tryout_id) if not current_user.can_manage_this_tryout(tryout): flash(_('Permission denied.'), 'danger') return redirect(url_for('tryouts.list_tryouts')) - player = User.query.get_or_404(player_id) + player = db.get_or_404(User, player_id) registration = TryoutRegistration.query.filter_by( tryout_id=tryout_id, player_id=player_id @@ -593,7 +593,7 @@ def remove_player(tryout_id, player_id): @login_required def create_team(tryout_id): """Create a tryout-specific team.""" - tryout = Tryout.query.get_or_404(tryout_id) + tryout = db.get_or_404(Tryout, tryout_id) if not current_user.can_manage_this_tryout(tryout): flash(_('Permission denied.'), 'danger') return redirect(url_for('tryouts.view_tryout', tryout_id=tryout_id)) @@ -615,8 +615,8 @@ def create_team(tryout_id): @login_required def add_to_team(tryout_id, team_id): """Add a player to a tryout team.""" - team = Team.query.get_or_404(team_id) - tryout = Tryout.query.get_or_404(tryout_id) + team = db.get_or_404(Team, team_id) + tryout = db.get_or_404(Tryout, tryout_id) if not current_user.can_manage_this_tryout(tryout): flash(_('Permission denied.'), 'danger') return redirect(url_for('tryouts.view_tryout', tryout_id=tryout_id)) @@ -658,7 +658,7 @@ def add_to_team(tryout_id, team_id): @login_required def delete_tryout(tryout_id): """Delete a tryout and all associated data (matches, teams, registrations, evaluations).""" - tryout = Tryout.query.get_or_404(tryout_id) + tryout = db.get_or_404(Tryout, tryout_id) if not current_user.can_manage_this_tryout(tryout): flash(_('You do not have permission to delete this tryout.'), 'danger') return redirect(url_for('tryouts.list_tryouts')) diff --git a/app/routes/users/accounts.py b/app/routes/users/accounts.py index c4dc991..1b64b82 100644 --- a/app/routes/users/accounts.py +++ b/app/routes/users/accounts.py @@ -70,7 +70,7 @@ def edit_user(user_id): flash(_('Only the president can edit users.'), 'danger') return redirect(url_for('main.dashboard')) - user = User.query.get_or_404(user_id) + user = db.get_or_404(User, user_id) if request.method == 'POST': actor_name, actor_id = current_user.username, current_user.id @@ -253,7 +253,7 @@ def delete_user(user_id): flash(_('You cannot delete your own account.'), 'danger') return redirect(url_for('users.list_users')) - user = User.query.get_or_404(user_id) + user = db.get_or_404(User, user_id) Evaluation.query.filter( db.or_(Evaluation.evaluator_id == user_id, Evaluation.player_id == user_id), @@ -379,5 +379,5 @@ def create_user(): @login_required def view_user(user_id): """View a public profile for any user.""" - user = User.query.get_or_404(user_id) + user = db.get_or_404(User, user_id) return render_template('pages/view_user.html', profile_user=user) diff --git a/app/routes/users/availability.py b/app/routes/users/availability.py index 6f30fb2..fcd3400 100644 --- a/app/routes/users/availability.py +++ b/app/routes/users/availability.py @@ -193,7 +193,7 @@ def clear_disponibilities(): @login_required def delete_disponibility(disponibility_id): """Delete a disponibility block.""" - disponibility = PlayerDisponibility.query.get_or_404(disponibility_id) + disponibility = db.get_or_404(PlayerDisponibility, disponibility_id) if disponibility.player_id != current_user.id: return jsonify({'error': 'Unauthorized'}), 403 db.session.delete(disponibility) diff --git a/app/routes/users/contracts.py b/app/routes/users/contracts.py index eda1c8b..ec9df0e 100644 --- a/app/routes/users/contracts.py +++ b/app/routes/users/contracts.py @@ -111,7 +111,7 @@ def upload_contract(): flash(error, 'danger') return redirect(url_for('users.upload_contract')) - player = User.query.get_or_404(player_id) + player = db.get_or_404(User, player_id) player_teams = player.get_org_teams() team = player_teams[0] if player_teams else None @@ -154,7 +154,7 @@ def upload_contract(): @login_required def upload_signed_contract(contract_id): """Upload a signed contract (player only).""" - contract = Contract.query.get_or_404(contract_id) + contract = db.get_or_404(Contract, contract_id) if not contract.can_upload_signed(current_user): flash(_('Only the player can upload their signed contract.'), 'danger') return redirect(url_for('users.list_contracts')) @@ -182,7 +182,7 @@ def upload_signed_contract(contract_id): @login_required def download_contract(contract_id): """Download a contract file.""" - contract = Contract.query.get_or_404(contract_id) + contract = db.get_or_404(Contract, contract_id) if not contract.can_view(current_user): flash(_('You do not have permission to download this contract.'), 'danger') return redirect(url_for('users.list_contracts')) @@ -197,7 +197,7 @@ def download_contract(contract_id): @login_required def download_signed_contract(contract_id): """Download a signed contract file.""" - contract = Contract.query.get_or_404(contract_id) + contract = db.get_or_404(Contract, contract_id) if not contract.can_view(current_user): flash(_('You do not have permission to download this contract.'), 'danger') return redirect(url_for('users.list_contracts')) diff --git a/app/routes/users/notes.py b/app/routes/users/notes.py index c69df65..2433e1e 100644 --- a/app/routes/users/notes.py +++ b/app/routes/users/notes.py @@ -217,7 +217,7 @@ def manage_personal_notes(): return redirect(url_for('users.notes_dashboard')) player_id = data['player_id'] - player = User.query.get_or_404(player_id) + player = db.get_or_404(User, player_id) if not isinstance(player, Player): flash(_('Can only add notes for players.'), 'danger') return redirect(url_for('users.notes_dashboard')) @@ -257,7 +257,7 @@ def add_personal_note(): return redirect(url_for('users.notes_dashboard')) player_id = data['player_id'] - player = User.query.get_or_404(player_id) + player = db.get_or_404(User, player_id) if not isinstance(player, Player): flash(_('Can only add notes for players.'), 'danger') return redirect(url_for('users.notes_dashboard')) @@ -267,7 +267,7 @@ def add_personal_note(): return redirect(url_for('users.notes_dashboard')) if data['match_id']: - match = Match.query.get_or_404(data['match_id']) + match = db.get_or_404(Match, data['match_id']) if not current_user.can_manage_this_tryout(match.tryout): flash(_('You cannot use that match as note context.'), 'danger') return redirect(url_for('users.notes_dashboard')) @@ -276,7 +276,7 @@ def add_personal_note(): return redirect(url_for('users.notes_dashboard')) if data['tryout_id']: - tryout = Tryout.query.get_or_404(data['tryout_id']) + tryout = db.get_or_404(Tryout, data['tryout_id']) if not current_user.can_manage_this_tryout(tryout): flash(_('You cannot use that tryout as note context.'), 'danger') return redirect(url_for('users.notes_dashboard')) @@ -285,7 +285,7 @@ def add_personal_note(): return redirect(url_for('users.notes_dashboard')) if data['team_id']: - team = Team.query.get_or_404(data['team_id']) + team = db.get_or_404(Team, data['team_id']) if not current_user.can_manage_this_tryout(team.tryout): flash(_('You cannot use that team as note context.'), 'danger') return redirect(url_for('users.notes_dashboard')) @@ -320,7 +320,7 @@ def add_note_from_tryout(tryout_id): flash(_('Only coaches can add personal notes.'), 'danger') return redirect(url_for('main.dashboard')) - tryout = Tryout.query.get_or_404(tryout_id) + tryout = db.get_or_404(Tryout, tryout_id) if not current_user.can_manage_this_tryout(tryout): flash(_('You do not have permission to add notes for this tryout.'), 'danger') return redirect(url_for('users.notes_dashboard')) @@ -384,7 +384,7 @@ def add_note_from_match(match_id): flash(_('Only coaches can add personal notes.'), 'danger') return redirect(url_for('main.dashboard')) - match_obj = Match.query.get_or_404(match_id) + match_obj = db.get_or_404(Match, match_id) if not current_user.can_manage_this_tryout(match_obj.tryout): flash(_('You do not have permission to add notes for this match.'), 'danger') return redirect(url_for('users.notes_dashboard')) diff --git a/app/routes/users/one_on_one.py b/app/routes/users/one_on_one.py index bccba76..cd3a612 100644 --- a/app/routes/users/one_on_one.py +++ b/app/routes/users/one_on_one.py @@ -166,7 +166,7 @@ def accept_one_on_one(request_id): flash(_('Only coaches can accept One on One requests.'), 'danger') return redirect(url_for('main.dashboard')) - request_obj = OneOnOneRequest.query.get_or_404(request_id) + request_obj = db.get_or_404(OneOnOneRequest, request_id) if request_obj.coach_id != current_user.id: flash(_('This request is not for you.'), 'danger') @@ -216,7 +216,7 @@ def reject_one_on_one(request_id): flash(_('Only coaches can reject One on One requests.'), 'danger') return redirect(url_for('main.dashboard')) - request_obj = OneOnOneRequest.query.get_or_404(request_id) + request_obj = db.get_or_404(OneOnOneRequest, request_id) if request_obj.coach_id != current_user.id: flash(_('This request is not for you.'), 'danger') diff --git a/tests/test_bot_error_families.py b/tests/test_bot_error_families.py index 919626c..8eba102 100644 --- a/tests/test_bot_error_families.py +++ b/tests/test_bot_error_families.py @@ -150,9 +150,10 @@ def _pending(bot, message_id, row_id): def _confirmed(row_id): + from app.extensions import db from app.models import MatchParticipant - return MatchParticipant.query.get(row_id).attendance_confirmed + return db.session.get(MatchParticipant, row_id).attendance_confirmed class TestTheDatabaseRefusedTheWrite: diff --git a/tests/test_query_shape.py b/tests/test_query_shape.py index 098d478..a8e1237 100644 --- a/tests/test_query_shape.py +++ b/tests/test_query_shape.py @@ -269,6 +269,78 @@ class TestRegisteredPlayersForMatchForm: assert counter.total == 1 +class TestEvaluationLists: + """Evaluation pages must not issue one lookup per player or evaluator.""" + + def test_players_to_evaluate_has_a_fixed_query_budget( + self, app, client, as_role, make_user, count_queries + ): + coach_id = as_role('coach') + admin_id = make_user('admin') + player_ids = [make_user('player') for _ in range(12)] + + with app.app_context(): + tryout = Tryout( + title='Evaluation budget', + game='Valorant', + date=date(2030, 3, 1), + created_by=admin_id, + coach_id=coach_id, + ) + db.session.add(tryout) + db.session.flush() + for player_id in player_ids: + db.session.add(TryoutRegistration(tryout_id=tryout.id, player_id=player_id)) + db.session.commit() + tryout_id = tryout.id + + counter = count_queries() + try: + response = client.get(f'/evaluations/{tryout_id}/players') + finally: + counter.stop() + + assert response.status_code == 200 + assert 1 <= counter.total <= 10, f'{counter.total} SELECTs for 12 players' + + def test_scout_top_players_are_loaded_with_the_aggregate( + self, app, client, as_role, make_user, count_queries + ): + as_role('scout') + evaluator_id = make_user('coach') + admin_id = make_user('admin') + player_ids = [make_user('player') for _ in range(12)] + + with app.app_context(): + tryout = Tryout( + title='Scout budget', + game='Valorant', + date=date(2030, 3, 1), + created_by=admin_id, + ) + db.session.add(tryout) + db.session.flush() + for score, player_id in enumerate(player_ids, start=1): + db.session.add( + Evaluation( + player_id=player_id, + evaluator_id=evaluator_id, + tryout_id=tryout.id, + overall_score=score, + ) + ) + db.session.commit() + + counter = count_queries() + try: + response = client.get('/dashboard') + finally: + counter.stop() + + assert response.status_code == 200 + assert 1 <= counter.total <= 6, f'{counter.total} SELECTs for the scout dashboard' + + class TestViewTryout: """PERF-001 — the most-visited page in the application ran one query per registration, one per player evaluated, one per team, and one per diff --git a/tests/test_storage.py b/tests/test_storage.py index 434c2be..e96197d 100644 --- a/tests/test_storage.py +++ b/tests/test_storage.py @@ -135,9 +135,11 @@ class TestThroughTheUploadRoute: contract_id = Contract.query.one().id response = client.get(f'/users/contracts/{contract_id}/download') - - assert response.status_code == 200 - assert response.data.startswith(b'%PDF-') + try: + assert response.status_code == 200 + assert response.data.startswith(b'%PDF-') + finally: + response.close() def _pdf():