diff --git a/README.md b/README.md index c2453c2..647510b 100644 --- a/README.md +++ b/README.md @@ -101,6 +101,12 @@ docker compose exec web python manage.py createsuperuser - `tailwind` service runs watch mode for development (`npm run dev`). - nginx proxies web traffic and serves static/media volume mounts. +## Search Consistency Notes + +- The server-rendered player search page (`/players/`) and read-only players API (`/api/players/`) use the same search form and ORM filter service. +- Sorting/filter semantics are aligned across UI, HTMX partial refreshes, and API responses. +- Statistical metric annotations are always computed for UI result tables, and only computed for API requests when metric-based sorting is requested. + ## Docker Volumes and Persistence `docker-compose.yml` uses named volumes: diff --git a/apps/api/serializers.py b/apps/api/serializers.py index 3eaddf3..1d12419 100644 --- a/apps/api/serializers.py +++ b/apps/api/serializers.py @@ -43,6 +43,8 @@ class PlayerListSerializer(serializers.ModelSerializer): nationality = serializers.CharField(source="nationality.name", allow_null=True) nominal_position = serializers.CharField(source="nominal_position.code", allow_null=True) inferred_role = serializers.CharField(source="inferred_role.name", allow_null=True) + origin_competition = serializers.CharField(source="origin_competition.name", allow_null=True) + origin_team = serializers.CharField(source="origin_team.name", allow_null=True) class Meta: model = Player @@ -53,6 +55,8 @@ class PlayerListSerializer(serializers.ModelSerializer): "nationality", "nominal_position", "inferred_role", + "origin_competition", + "origin_team", "height_cm", "weight_kg", "dominant_hand", @@ -88,6 +92,8 @@ class PlayerDetailSerializer(serializers.ModelSerializer): nationality = serializers.CharField(source="nationality.name", allow_null=True) nominal_position = serializers.CharField(source="nominal_position.name", allow_null=True) inferred_role = serializers.CharField(source="inferred_role.name", allow_null=True) + origin_competition = serializers.CharField(source="origin_competition.name", allow_null=True) + origin_team = serializers.CharField(source="origin_team.name", allow_null=True) age = serializers.SerializerMethodField() aliases = serializers.SerializerMethodField() season_stats = serializers.SerializerMethodField() @@ -102,6 +108,8 @@ class PlayerDetailSerializer(serializers.ModelSerializer): "nationality", "nominal_position", "inferred_role", + "origin_competition", + "origin_team", "height_cm", "weight_kg", "dominant_hand", diff --git a/apps/api/views.py b/apps/api/views.py index dc4d64e..14ddd33 100644 --- a/apps/api/views.py +++ b/apps/api/views.py @@ -5,7 +5,13 @@ from rest_framework.throttling import AnonRateThrottle, UserRateThrottle from apps.competitions.models import Competition, Season from apps.players.forms import PlayerSearchForm from apps.players.models import Player -from apps.players.services.search import apply_sorting, base_player_queryset, filter_players +from apps.players.services.search import ( + METRIC_SORT_KEYS, + annotate_player_metrics, + apply_sorting, + base_player_queryset, + filter_players, +) from apps.teams.models import Team from .permissions import ReadOnlyOrDeny @@ -38,7 +44,10 @@ class PlayerSearchApiView(ReadOnlyBaseAPIView, generics.ListAPIView): queryset = base_player_queryset() if form.is_valid(): queryset = filter_players(queryset, form.cleaned_data) - queryset = apply_sorting(queryset, form.cleaned_data.get("sort", "name_asc")) + sort_key = form.cleaned_data.get("sort", "name_asc") + if sort_key in METRIC_SORT_KEYS: + queryset = annotate_player_metrics(queryset) + queryset = apply_sorting(queryset, sort_key) else: queryset = queryset.order_by("full_name", "id") return queryset @@ -50,6 +59,8 @@ class PlayerDetailApiView(ReadOnlyBaseAPIView, generics.RetrieveAPIView): "nationality", "nominal_position", "inferred_role", + "origin_competition", + "origin_team", ).prefetch_related("aliases") diff --git a/apps/players/migrations/0005_player_weight_index.py b/apps/players/migrations/0005_player_weight_index.py new file mode 100644 index 0000000..f404876 --- /dev/null +++ b/apps/players/migrations/0005_player_weight_index.py @@ -0,0 +1,17 @@ +# Generated by Django 5.2.12 on 2026-03-10 17:05 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("players", "0004_backfill_player_origins"), + ] + + operations = [ + migrations.AddIndex( + model_name="player", + index=models.Index(fields=["weight_kg"], name="players_pla_weight__fb76a4_idx"), + ), + ] diff --git a/apps/players/models.py b/apps/players/models.py index 87ed802..12cd671 100644 --- a/apps/players/models.py +++ b/apps/players/models.py @@ -123,6 +123,7 @@ class Player(TimeStampedModel): models.Index(fields=["origin_team"]), models.Index(fields=["is_active"]), models.Index(fields=["height_cm"]), + models.Index(fields=["weight_kg"]), ] def __str__(self) -> str: diff --git a/apps/players/services/search.py b/apps/players/services/search.py index ad3656e..09d18af 100644 --- a/apps/players/services/search.py +++ b/apps/players/services/search.py @@ -16,6 +16,8 @@ from django.db.models.functions import Coalesce from apps.players.models import Player +METRIC_SORT_KEYS = {"ppg_desc", "ppg_asc", "mpg_desc", "mpg_asc"} + def _years_ago_today(years: int) -> date: today = date.today() @@ -36,6 +38,40 @@ def _apply_min_max_filter(queryset, min_key: str, max_key: str, field_name: str, return queryset +def _needs_distinct(data: dict) -> bool: + join_filter_keys = ( + "q", + "team", + "competition", + "season", + "games_played_min", + "games_played_max", + "minutes_per_game_min", + "minutes_per_game_max", + "points_per_game_min", + "points_per_game_max", + "rebounds_per_game_min", + "rebounds_per_game_max", + "assists_per_game_min", + "assists_per_game_max", + "steals_per_game_min", + "steals_per_game_max", + "blocks_per_game_min", + "blocks_per_game_max", + "turnovers_per_game_min", + "turnovers_per_game_max", + "fg_pct_min", + "fg_pct_max", + "three_pct_min", + "three_pct_max", + "ft_pct_min", + "ft_pct_max", + "efficiency_metric_min", + "efficiency_metric_max", + ) + return any(data.get(key) not in (None, "") for key in join_filter_keys) + + def filter_players(queryset, data: dict): query = data.get("q") if query: @@ -108,6 +144,12 @@ def filter_players(queryset, data: dict): for min_key, max_key, field_name in stat_pairs: queryset = _apply_min_max_filter(queryset, min_key, max_key, field_name, data) + if _needs_distinct(data): + return queryset.distinct() + return queryset + + +def annotate_player_metrics(queryset): mpg_expression = Case( When( player_seasons__games_played__gt=0, @@ -120,7 +162,7 @@ def filter_players(queryset, data: dict): output_field=FloatField(), ) - queryset = queryset.annotate( + return queryset.annotate( games_played_value=Coalesce( Max("player_seasons__games_played"), Value(0, output_field=IntegerField()), @@ -159,8 +201,6 @@ def filter_players(queryset, data: dict): ), ) - return queryset.distinct() - def apply_sorting(queryset, sort_key: str): if sort_key == "name_desc": @@ -191,4 +231,4 @@ def base_player_queryset(): "inferred_role", "origin_competition", "origin_team", - ).prefetch_related("aliases") + ) diff --git a/apps/players/views.py b/apps/players/views.py index 8aac4ad..37a118e 100644 --- a/apps/players/views.py +++ b/apps/players/views.py @@ -7,8 +7,8 @@ from apps.scouting.models import FavoritePlayer from apps.stats.models import PlayerSeason from .forms import PlayerSearchForm -from .models import Player -from .services.search import apply_sorting, base_player_queryset, filter_players +from .models import Player, PlayerCareerEntry +from .services.search import annotate_player_metrics, apply_sorting, base_player_queryset, filter_players def calculate_age(birth_date): @@ -48,9 +48,10 @@ class PlayerSearchView(ListView): if form.is_valid(): queryset = filter_players(queryset, form.cleaned_data) + queryset = annotate_player_metrics(queryset) queryset = apply_sorting(queryset, form.cleaned_data.get("sort", "name_asc")) else: - queryset = queryset.order_by("full_name", "id") + queryset = annotate_player_metrics(queryset).order_by("full_name", "id") return queryset @@ -81,6 +82,12 @@ class PlayerDetailView(DetailView): "competition", "stats", ).order_by("-season__start_date", "-id") + career_queryset = PlayerCareerEntry.objects.select_related( + "team", + "competition", + "season", + "role_snapshot", + ).order_by("-start_date", "-id") return ( Player.objects.select_related( @@ -93,10 +100,7 @@ class PlayerDetailView(DetailView): .prefetch_related( "aliases", Prefetch("player_seasons", queryset=season_queryset), - "career_entries__team", - "career_entries__competition", - "career_entries__season", - "career_entries__role_snapshot", + Prefetch("career_entries", queryset=career_queryset), ) ) @@ -132,7 +136,7 @@ class PlayerDetailView(DetailView): context["age"] = calculate_age(player.birth_date) context["current_assignment"] = current_assignment - context["career_entries"] = player.career_entries.all().order_by("-start_date", "-id") + context["career_entries"] = player.career_entries.all() context["season_rows"] = season_rows context["is_favorite"] = False if self.request.user.is_authenticated: diff --git a/apps/stats/migrations/0002_playerseasonstats_search_indexes.py b/apps/stats/migrations/0002_playerseasonstats_search_indexes.py new file mode 100644 index 0000000..b491c56 --- /dev/null +++ b/apps/stats/migrations/0002_playerseasonstats_search_indexes.py @@ -0,0 +1,41 @@ +# Generated by Django 5.2.12 on 2026-03-10 17:05 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("stats", "0001_initial"), + ] + + operations = [ + migrations.AddIndex( + model_name="playerseasonstats", + index=models.Index(fields=["steals"], name="stats_playe_steals_59b0f3_idx"), + ), + migrations.AddIndex( + model_name="playerseasonstats", + index=models.Index(fields=["blocks"], name="stats_playe_blocks_b2d4de_idx"), + ), + migrations.AddIndex( + model_name="playerseasonstats", + index=models.Index(fields=["turnovers"], name="stats_playe_turnove_aa4e87_idx"), + ), + migrations.AddIndex( + model_name="playerseasonstats", + index=models.Index(fields=["fg_pct"], name="stats_playe_fg_pct_bf2ff1_idx"), + ), + migrations.AddIndex( + model_name="playerseasonstats", + index=models.Index(fields=["three_pct"], name="stats_playe_three_p_c67201_idx"), + ), + migrations.AddIndex( + model_name="playerseasonstats", + index=models.Index(fields=["ft_pct"], name="stats_playe_ft_pct_da7421_idx"), + ), + migrations.AddIndex( + model_name="playerseasonstats", + index=models.Index(fields=["player_efficiency_rating"], name="stats_playe_player__641815_idx"), + ), + ] diff --git a/apps/stats/models.py b/apps/stats/models.py index b7a3885..27f382c 100644 --- a/apps/stats/models.py +++ b/apps/stats/models.py @@ -63,8 +63,15 @@ class PlayerSeasonStats(models.Model): models.Index(fields=["points"]), models.Index(fields=["rebounds"]), models.Index(fields=["assists"]), + models.Index(fields=["steals"]), + models.Index(fields=["blocks"]), + models.Index(fields=["turnovers"]), + models.Index(fields=["fg_pct"]), + models.Index(fields=["three_pct"]), + models.Index(fields=["ft_pct"]), models.Index(fields=["usage_rate"]), models.Index(fields=["true_shooting_pct"]), + models.Index(fields=["player_efficiency_rating"]), ] def __str__(self) -> str: diff --git a/tests/test_api.py b/tests/test_api.py index ba3b803..5bde9b2 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -5,6 +5,7 @@ from django.urls import reverse from apps.competitions.models import Competition, Season from apps.players.models import Nationality, Player, Position, Role +from apps.stats.models import PlayerSeason, PlayerSeasonStats from apps.teams.models import Team @@ -57,3 +58,99 @@ def test_lookup_list_endpoints(client): def test_api_is_read_only(client): response = client.post(reverse("api:players"), data={"q": "x"}) assert response.status_code == 403 + + +@pytest.mark.django_db +def test_players_api_search_consistent_with_ui_filters(client): + nationality = Nationality.objects.create(name="Portugal", iso2_code="PT", iso3_code="PRT") + position = Position.objects.create(code="SF", name="Small Forward") + role = Role.objects.create(code="wing", name="Wing") + competition = Competition.objects.create( + name="Liga Betclic", + slug="liga-betclic", + competition_type=Competition.CompetitionType.LEAGUE, + gender=Competition.Gender.MEN, + country=nationality, + ) + team = Team.objects.create(name="Porto Hoops", slug="porto-hoops", country=nationality) + season = Season.objects.create(label="2025-2026", start_date=date(2025, 9, 1), end_date=date(2026, 6, 30)) + + matching = Player.objects.create( + first_name="Tiago", + last_name="Silva", + full_name="Tiago Silva", + birth_date=date(2001, 3, 1), + nationality=nationality, + nominal_position=position, + inferred_role=role, + origin_competition=competition, + origin_team=team, + ) + ps = PlayerSeason.objects.create( + player=matching, + season=season, + team=team, + competition=competition, + games_played=10, + minutes_played=320, + ) + PlayerSeasonStats.objects.create( + player_season=ps, + points=16.5, + rebounds=5, + assists=3, + steals=1, + blocks=0.5, + turnovers=2, + ) + + Player.objects.create( + first_name="Pedro", + last_name="Costa", + full_name="Pedro Costa", + birth_date=date(2001, 4, 2), + nationality=nationality, + ) + + params = { + "origin_competition": competition.id, + "nominal_position": position.id, + "points_per_game_min": "10", + "sort": "ppg_desc", + } + ui_response = client.get(reverse("players:index"), data=params) + api_response = client.get(reverse("api:players"), data=params) + + assert ui_response.status_code == 200 + assert api_response.status_code == 200 + assert list(ui_response.context["players"])[0].id == matching.id + assert api_response.json()["count"] == 1 + assert api_response.json()["results"][0]["id"] == matching.id + + +@pytest.mark.django_db +def test_player_detail_api_includes_origin_fields(client): + nationality = Nationality.objects.create(name="Greece", iso2_code="GR", iso3_code="GRC") + competition = Competition.objects.create( + name="HEBA A1", + slug="heba-a1", + competition_type=Competition.CompetitionType.LEAGUE, + gender=Competition.Gender.MEN, + country=nationality, + ) + team = Team.objects.create(name="Athens BC", slug="athens-bc", country=nationality) + player = Player.objects.create( + first_name="Alex", + last_name="Dimitriou", + full_name="Alex Dimitriou", + birth_date=date(2000, 2, 2), + nationality=nationality, + origin_competition=competition, + origin_team=team, + ) + + response = client.get(reverse("api:player_detail", kwargs={"pk": player.pk})) + assert response.status_code == 200 + payload = response.json() + assert payload["origin_competition"] == competition.name + assert payload["origin_team"] == team.name diff --git a/tests/test_integration_paths.py b/tests/test_integration_paths.py index 742dd00..a4f4599 100644 --- a/tests/test_integration_paths.py +++ b/tests/test_integration_paths.py @@ -4,6 +4,8 @@ import pytest from django.contrib.auth.models import User from django.urls import reverse +from apps.ingestion.models import IngestionRun +from apps.ingestion.services.sync import run_sync_job from apps.players.models import Nationality, Player, Position, Role from apps.scouting.models import SavedSearch @@ -47,3 +49,25 @@ def test_saved_search_run_filters_player_results(client): assert response.status_code == 200 assert "Marco Rossi" in response.content.decode() assert "Luca Bianchi" not in response.content.decode() + + +@pytest.mark.django_db +def test_ingestion_output_is_searchable_in_ui_and_api(settings, client): + settings.PROVIDER_DEFAULT_NAMESPACE = "mvp_demo" + run = run_sync_job(provider_namespace="mvp_demo", job_type=IngestionRun.JobType.FULL_SYNC) + assert run.status == IngestionRun.RunStatus.SUCCESS + + player = Player.objects.filter(origin_competition__isnull=False).order_by("id").first() + assert player is not None + assert player.origin_competition_id is not None + + params = {"origin_competition": player.origin_competition_id} + ui_response = client.get(reverse("players:index"), data=params) + api_response = client.get(reverse("api:players"), data=params) + + assert ui_response.status_code == 200 + assert api_response.status_code == 200 + ui_ids = {item.id for item in ui_response.context["players"]} + api_ids = {item["id"] for item in api_response.json()["results"]} + assert player.id in ui_ids + assert player.id in api_ids diff --git a/tests/test_players_filters_advanced.py b/tests/test_players_filters_advanced.py index 6a5c362..f5d1ae0 100644 --- a/tests/test_players_filters_advanced.py +++ b/tests/test_players_filters_advanced.py @@ -1,10 +1,12 @@ from datetime import date import pytest +from django.contrib.auth.models import User from django.urls import reverse from apps.competitions.models import Competition, Season from apps.players.models import Nationality, Player, Position, Role +from apps.scouting.models import FavoritePlayer from apps.stats.models import PlayerSeason, PlayerSeasonStats from apps.teams.models import Team @@ -81,3 +83,106 @@ def test_player_search_pagination_preserves_querystring(client): assert response.status_code == 200 assert response.context["page_obj"].number == 2 + + +@pytest.mark.django_db +def test_player_search_combined_filters_sorting_and_pagination(client): + nationality = Nationality.objects.create(name="Serbia", iso2_code="RS", iso3_code="SRB") + position = Position.objects.create(code="SG", name="Shooting Guard") + role = Role.objects.create(code="scorer", name="Scorer") + season = Season.objects.create(label="2024-2025", start_date=date(2024, 9, 1), end_date=date(2025, 6, 30)) + competition = Competition.objects.create( + name="ABA League", + slug="aba-league", + competition_type=Competition.CompetitionType.LEAGUE, + gender=Competition.Gender.MEN, + country=nationality, + ) + team = Team.objects.create(name="Belgrade BC", slug="belgrade-bc", country=nationality) + + players = [] + for idx, ppg in enumerate(range(40, 19, -1), start=1): + player = Player.objects.create( + first_name=f"S{idx}", + last_name=f"Guard{idx}", + full_name=f"Serbian Guard {idx}", + birth_date=date(2001, 1, idx), + nationality=nationality, + nominal_position=position, + inferred_role=role, + origin_competition=competition, + origin_team=team, + ) + player_season = PlayerSeason.objects.create( + player=player, + season=season, + team=team, + competition=competition, + games_played=20, + minutes_played=600, + ) + PlayerSeasonStats.objects.create( + player_season=player_season, + points=ppg, + rebounds=4.0, + assists=3.0, + steals=1.0, + blocks=0.3, + turnovers=2.0, + ) + players.append(player) + + response = client.get( + reverse("players:index"), + data={ + "origin_competition": competition.id, + "nominal_position": position.id, + "sort": "ppg_desc", + "page_size": 20, + "page": 1, + }, + ) + + assert response.status_code == 200 + page_items = list(response.context["players"]) + assert len(page_items) == 20 + assert page_items[0].full_name == players[0].full_name + assert response.context["page_obj"].has_next() + + page2 = client.get( + reverse("players:index"), + data={ + "origin_competition": competition.id, + "nominal_position": position.id, + "sort": "ppg_desc", + "page_size": 20, + "page": 2, + }, + ) + assert page2.status_code == 200 + page2_items = list(page2.context["players"]) + assert [item.full_name for item in page2_items] == [players[20].full_name] + + +@pytest.mark.django_db +def test_player_search_results_include_favorite_ids(client): + user = User.objects.create_user(username="fav-check", password="pass12345") + client.force_login(user) + + nationality = Nationality.objects.create(name="Croatia", iso2_code="HR", iso3_code="HRV") + position = Position.objects.create(code="PG", name="Point Guard") + role = Role.objects.create(code="playmaker", name="Playmaker") + player = Player.objects.create( + first_name="Niko", + last_name="Play", + full_name="Niko Play", + birth_date=date(2002, 5, 5), + nationality=nationality, + nominal_position=position, + inferred_role=role, + ) + FavoritePlayer.objects.create(user=user, player=player) + + response = client.get(reverse("players:index")) + assert response.status_code == 200 + assert player.id in response.context["favorite_player_ids"]