Improve search quality, ORM efficiency, and filter consistency

This commit is contained in:
Alfredo Di Stasio
2026-03-10 14:37:01 +01:00
parent ceff4bc42c
commit a1ae380fd5
12 changed files with 375 additions and 14 deletions

View File

@ -101,6 +101,12 @@ docker compose exec web python manage.py createsuperuser
- `tailwind` service runs watch mode for development (`npm run dev`). - `tailwind` service runs watch mode for development (`npm run dev`).
- nginx proxies web traffic and serves static/media volume mounts. - 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 Volumes and Persistence
`docker-compose.yml` uses named volumes: `docker-compose.yml` uses named volumes:

View File

@ -43,6 +43,8 @@ class PlayerListSerializer(serializers.ModelSerializer):
nationality = serializers.CharField(source="nationality.name", allow_null=True) nationality = serializers.CharField(source="nationality.name", allow_null=True)
nominal_position = serializers.CharField(source="nominal_position.code", 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) 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: class Meta:
model = Player model = Player
@ -53,6 +55,8 @@ class PlayerListSerializer(serializers.ModelSerializer):
"nationality", "nationality",
"nominal_position", "nominal_position",
"inferred_role", "inferred_role",
"origin_competition",
"origin_team",
"height_cm", "height_cm",
"weight_kg", "weight_kg",
"dominant_hand", "dominant_hand",
@ -88,6 +92,8 @@ class PlayerDetailSerializer(serializers.ModelSerializer):
nationality = serializers.CharField(source="nationality.name", allow_null=True) nationality = serializers.CharField(source="nationality.name", allow_null=True)
nominal_position = serializers.CharField(source="nominal_position.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) 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() age = serializers.SerializerMethodField()
aliases = serializers.SerializerMethodField() aliases = serializers.SerializerMethodField()
season_stats = serializers.SerializerMethodField() season_stats = serializers.SerializerMethodField()
@ -102,6 +108,8 @@ class PlayerDetailSerializer(serializers.ModelSerializer):
"nationality", "nationality",
"nominal_position", "nominal_position",
"inferred_role", "inferred_role",
"origin_competition",
"origin_team",
"height_cm", "height_cm",
"weight_kg", "weight_kg",
"dominant_hand", "dominant_hand",

View File

@ -5,7 +5,13 @@ from rest_framework.throttling import AnonRateThrottle, UserRateThrottle
from apps.competitions.models import Competition, Season from apps.competitions.models import Competition, Season
from apps.players.forms import PlayerSearchForm from apps.players.forms import PlayerSearchForm
from apps.players.models import Player 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 apps.teams.models import Team
from .permissions import ReadOnlyOrDeny from .permissions import ReadOnlyOrDeny
@ -38,7 +44,10 @@ class PlayerSearchApiView(ReadOnlyBaseAPIView, generics.ListAPIView):
queryset = base_player_queryset() queryset = base_player_queryset()
if form.is_valid(): if form.is_valid():
queryset = filter_players(queryset, form.cleaned_data) 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: else:
queryset = queryset.order_by("full_name", "id") queryset = queryset.order_by("full_name", "id")
return queryset return queryset
@ -50,6 +59,8 @@ class PlayerDetailApiView(ReadOnlyBaseAPIView, generics.RetrieveAPIView):
"nationality", "nationality",
"nominal_position", "nominal_position",
"inferred_role", "inferred_role",
"origin_competition",
"origin_team",
).prefetch_related("aliases") ).prefetch_related("aliases")

View File

@ -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"),
),
]

View File

@ -123,6 +123,7 @@ class Player(TimeStampedModel):
models.Index(fields=["origin_team"]), models.Index(fields=["origin_team"]),
models.Index(fields=["is_active"]), models.Index(fields=["is_active"]),
models.Index(fields=["height_cm"]), models.Index(fields=["height_cm"]),
models.Index(fields=["weight_kg"]),
] ]
def __str__(self) -> str: def __str__(self) -> str:

View File

@ -16,6 +16,8 @@ from django.db.models.functions import Coalesce
from apps.players.models import Player from apps.players.models import Player
METRIC_SORT_KEYS = {"ppg_desc", "ppg_asc", "mpg_desc", "mpg_asc"}
def _years_ago_today(years: int) -> date: def _years_ago_today(years: int) -> date:
today = date.today() today = date.today()
@ -36,6 +38,40 @@ def _apply_min_max_filter(queryset, min_key: str, max_key: str, field_name: str,
return queryset 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): def filter_players(queryset, data: dict):
query = data.get("q") query = data.get("q")
if query: if query:
@ -108,6 +144,12 @@ def filter_players(queryset, data: dict):
for min_key, max_key, field_name in stat_pairs: for min_key, max_key, field_name in stat_pairs:
queryset = _apply_min_max_filter(queryset, min_key, max_key, field_name, data) 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( mpg_expression = Case(
When( When(
player_seasons__games_played__gt=0, player_seasons__games_played__gt=0,
@ -120,7 +162,7 @@ def filter_players(queryset, data: dict):
output_field=FloatField(), output_field=FloatField(),
) )
queryset = queryset.annotate( return queryset.annotate(
games_played_value=Coalesce( games_played_value=Coalesce(
Max("player_seasons__games_played"), Max("player_seasons__games_played"),
Value(0, output_field=IntegerField()), 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): def apply_sorting(queryset, sort_key: str):
if sort_key == "name_desc": if sort_key == "name_desc":
@ -191,4 +231,4 @@ def base_player_queryset():
"inferred_role", "inferred_role",
"origin_competition", "origin_competition",
"origin_team", "origin_team",
).prefetch_related("aliases") )

View File

@ -7,8 +7,8 @@ from apps.scouting.models import FavoritePlayer
from apps.stats.models import PlayerSeason from apps.stats.models import PlayerSeason
from .forms import PlayerSearchForm from .forms import PlayerSearchForm
from .models import Player from .models import Player, PlayerCareerEntry
from .services.search import apply_sorting, base_player_queryset, filter_players from .services.search import annotate_player_metrics, apply_sorting, base_player_queryset, filter_players
def calculate_age(birth_date): def calculate_age(birth_date):
@ -48,9 +48,10 @@ class PlayerSearchView(ListView):
if form.is_valid(): if form.is_valid():
queryset = filter_players(queryset, form.cleaned_data) queryset = filter_players(queryset, form.cleaned_data)
queryset = annotate_player_metrics(queryset)
queryset = apply_sorting(queryset, form.cleaned_data.get("sort", "name_asc")) queryset = apply_sorting(queryset, form.cleaned_data.get("sort", "name_asc"))
else: else:
queryset = queryset.order_by("full_name", "id") queryset = annotate_player_metrics(queryset).order_by("full_name", "id")
return queryset return queryset
@ -81,6 +82,12 @@ class PlayerDetailView(DetailView):
"competition", "competition",
"stats", "stats",
).order_by("-season__start_date", "-id") ).order_by("-season__start_date", "-id")
career_queryset = PlayerCareerEntry.objects.select_related(
"team",
"competition",
"season",
"role_snapshot",
).order_by("-start_date", "-id")
return ( return (
Player.objects.select_related( Player.objects.select_related(
@ -93,10 +100,7 @@ class PlayerDetailView(DetailView):
.prefetch_related( .prefetch_related(
"aliases", "aliases",
Prefetch("player_seasons", queryset=season_queryset), Prefetch("player_seasons", queryset=season_queryset),
"career_entries__team", Prefetch("career_entries", queryset=career_queryset),
"career_entries__competition",
"career_entries__season",
"career_entries__role_snapshot",
) )
) )
@ -132,7 +136,7 @@ class PlayerDetailView(DetailView):
context["age"] = calculate_age(player.birth_date) context["age"] = calculate_age(player.birth_date)
context["current_assignment"] = current_assignment 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["season_rows"] = season_rows
context["is_favorite"] = False context["is_favorite"] = False
if self.request.user.is_authenticated: if self.request.user.is_authenticated:

View File

@ -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"),
),
]

View File

@ -63,8 +63,15 @@ class PlayerSeasonStats(models.Model):
models.Index(fields=["points"]), models.Index(fields=["points"]),
models.Index(fields=["rebounds"]), models.Index(fields=["rebounds"]),
models.Index(fields=["assists"]), 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=["usage_rate"]),
models.Index(fields=["true_shooting_pct"]), models.Index(fields=["true_shooting_pct"]),
models.Index(fields=["player_efficiency_rating"]),
] ]
def __str__(self) -> str: def __str__(self) -> str:

View File

@ -5,6 +5,7 @@ from django.urls import reverse
from apps.competitions.models import Competition, Season from apps.competitions.models import Competition, Season
from apps.players.models import Nationality, Player, Position, Role from apps.players.models import Nationality, Player, Position, Role
from apps.stats.models import PlayerSeason, PlayerSeasonStats
from apps.teams.models import Team from apps.teams.models import Team
@ -57,3 +58,99 @@ def test_lookup_list_endpoints(client):
def test_api_is_read_only(client): def test_api_is_read_only(client):
response = client.post(reverse("api:players"), data={"q": "x"}) response = client.post(reverse("api:players"), data={"q": "x"})
assert response.status_code == 403 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

View File

@ -4,6 +4,8 @@ import pytest
from django.contrib.auth.models import User from django.contrib.auth.models import User
from django.urls import reverse 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.players.models import Nationality, Player, Position, Role
from apps.scouting.models import SavedSearch 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 response.status_code == 200
assert "Marco Rossi" in response.content.decode() assert "Marco Rossi" in response.content.decode()
assert "Luca Bianchi" not 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

View File

@ -1,10 +1,12 @@
from datetime import date from datetime import date
import pytest import pytest
from django.contrib.auth.models import User
from django.urls import reverse from django.urls import reverse
from apps.competitions.models import Competition, Season from apps.competitions.models import Competition, Season
from apps.players.models import Nationality, Player, Position, Role from apps.players.models import Nationality, Player, Position, Role
from apps.scouting.models import FavoritePlayer
from apps.stats.models import PlayerSeason, PlayerSeasonStats from apps.stats.models import PlayerSeason, PlayerSeasonStats
from apps.teams.models import Team 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.status_code == 200
assert response.context["page_obj"].number == 2 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"]