From 6fc583c79f9e170ac129601feb0beb235b6a7367 Mon Sep 17 00:00:00 2001 From: Alfredo Di Stasio Date: Fri, 13 Mar 2026 14:10:39 +0100 Subject: [PATCH] feat(v2): implement scoped player search and detail flows --- README.md | 26 +++++++++ apps/players/forms.py | 19 ------ apps/players/services/search.py | 77 +++++++++++++------------ apps/players/views.py | 11 +--- templates/players/detail.html | 42 ++------------ templates/players/index.html | 6 +- templates/players/partials/results.html | 27 +++++---- tests/test_api.py | 4 +- tests/test_players_filters_advanced.py | 6 +- tests/test_players_views.py | 48 +++++++++++++-- 10 files changed, 137 insertions(+), 129 deletions(-) diff --git a/README.md b/README.md index 27a83f3..d8d0fec 100644 --- a/README.md +++ b/README.md @@ -181,6 +181,32 @@ docker compose exec web python manage.py createsuperuser - app health: `/health/` - nginx healthcheck proxies `/health/` to `web` +## Player Search (v2) + +Public player search is server-rendered (Django templates) with HTMX partial updates. + +Supported filters: +- free text name search +- nominal position, inferred role +- competition, season, team +- nationality +- age, height, weight ranges +- stats thresholds: games, MPG, PPG, RPG, APG, SPG, BPG, TOV, FG%, 3P%, FT% + +Search correctness: +- combined team/competition/season/stat filters are applied to the same `PlayerSeason` context (no cross-row false positives) +- filtering happens at database level with Django ORM + +Search metric semantics: +- result columns are labeled as **Best Eligible** +- each displayed metric is `MAX` over eligible player-season rows for that metric in the current filter context +- different metric columns for one player may come from different eligible seasons +- when no eligible value exists for a metric in the current context, the UI shows `-` + +Pagination and sorting: +- querystring is preserved +- HTMX navigation keeps URL state in sync with current filters/page/sort + ## GitFlow Required branch model: diff --git a/apps/players/forms.py b/apps/players/forms.py index f7a0cfe..96dca1a 100644 --- a/apps/players/forms.py +++ b/apps/players/forms.py @@ -25,10 +25,8 @@ class PlayerSearchForm(forms.Form): nominal_position = forms.ModelChoiceField(queryset=Position.objects.none(), required=False) inferred_role = forms.ModelChoiceField(queryset=Role.objects.none(), required=False) competition = forms.ModelChoiceField(queryset=Competition.objects.none(), required=False) - origin_competition = forms.ModelChoiceField(queryset=Competition.objects.none(), required=False) nationality = forms.ModelChoiceField(queryset=Nationality.objects.none(), required=False) team = forms.ModelChoiceField(queryset=Team.objects.none(), required=False) - origin_team = forms.ModelChoiceField(queryset=Team.objects.none(), required=False) season = forms.ModelChoiceField(queryset=Season.objects.none(), required=False) age_min = forms.IntegerField(required=False, min_value=0, max_value=60, label="Min age") @@ -60,20 +58,6 @@ class PlayerSearchForm(forms.Form): three_pct_max = forms.DecimalField(required=False, min_value=0, decimal_places=2, max_digits=5, label="3P% max") ft_pct_min = forms.DecimalField(required=False, min_value=0, decimal_places=2, max_digits=5, label="FT% min") ft_pct_max = forms.DecimalField(required=False, min_value=0, decimal_places=2, max_digits=5, label="FT% max") - efficiency_metric_min = forms.DecimalField( - required=False, - min_value=0, - decimal_places=2, - max_digits=6, - label="Impact metric min", - ) - efficiency_metric_max = forms.DecimalField( - required=False, - min_value=0, - decimal_places=2, - max_digits=6, - label="Impact metric max", - ) sort = forms.ChoiceField(choices=SORT_CHOICES, required=False, initial="name_asc") page_size = forms.TypedChoiceField( @@ -88,10 +72,8 @@ class PlayerSearchForm(forms.Form): self.fields["nominal_position"].queryset = Position.objects.order_by("code") self.fields["inferred_role"].queryset = Role.objects.order_by("name") self.fields["competition"].queryset = Competition.objects.order_by("name") - self.fields["origin_competition"].queryset = Competition.objects.order_by("name") self.fields["nationality"].queryset = Nationality.objects.order_by("name") self.fields["team"].queryset = Team.objects.order_by("name") - self.fields["origin_team"].queryset = Team.objects.order_by("name") self.fields["season"].queryset = Season.objects.order_by("-start_date") def clean(self): @@ -110,7 +92,6 @@ class PlayerSearchForm(forms.Form): self._validate_min_max(cleaned_data, "fg_pct_min", "fg_pct_max") self._validate_min_max(cleaned_data, "three_pct_min", "three_pct_max") self._validate_min_max(cleaned_data, "ft_pct_min", "ft_pct_max") - self._validate_min_max(cleaned_data, "efficiency_metric_min", "efficiency_metric_max") if not cleaned_data.get("sort"): cleaned_data["sort"] = "name_asc" diff --git a/apps/players/services/search.py b/apps/players/services/search.py index 31fcea8..2a1d596 100644 --- a/apps/players/services/search.py +++ b/apps/players/services/search.py @@ -14,7 +14,6 @@ from django.db.models import ( Value, When, ) -from django.db.models.functions import Coalesce from apps.players.models import Player from apps.stats.models import PlayerSeason @@ -22,7 +21,8 @@ from apps.stats.models import PlayerSeason METRIC_SORT_KEYS = {"ppg_desc", "ppg_asc", "mpg_desc", "mpg_asc"} SEARCH_METRIC_SEMANTICS_TEXT = ( "Search metrics are best eligible values per player (max per metric across eligible player-season rows). " - "With season/team/competition/stat filters, eligibility is scoped by those filters." + "With season/team/competition/stat filters, eligibility is scoped by those filters. " + "When no eligible stat exists in the current filter context, metric cells show '-'." ) @@ -73,8 +73,6 @@ def _season_scope_filter_keys() -> tuple[str, ...]: "three_pct_max", "ft_pct_min", "ft_pct_max", - "efficiency_metric_min", - "efficiency_metric_max", ) @@ -121,7 +119,6 @@ def _apply_player_season_scope_filters(queryset, data: dict): ("fg_pct_min", "fg_pct_max", "stats__fg_pct"), ("three_pct_min", "three_pct_max", "stats__three_pct"), ("ft_pct_min", "ft_pct_max", "stats__ft_pct"), - ("efficiency_metric_min", "efficiency_metric_max", "stats__player_efficiency_rating"), ) for min_key, max_key, field_name in stat_pairs: queryset = _apply_min_max_filter(queryset, min_key, max_key, field_name, data) @@ -149,11 +146,6 @@ def _build_metric_context_filter(data: dict) -> Q: ("fg_pct_min", "fg_pct_max", "player_seasons__stats__fg_pct"), ("three_pct_min", "three_pct_max", "player_seasons__stats__three_pct"), ("ft_pct_min", "ft_pct_max", "player_seasons__stats__ft_pct"), - ( - "efficiency_metric_min", - "efficiency_metric_max", - "player_seasons__stats__player_efficiency_rating", - ), ) for min_key, max_key, field_name in minmax_pairs: min_value = data.get(min_key) @@ -188,10 +180,6 @@ def filter_players(queryset, data: dict): queryset = queryset.filter(inferred_role=data["inferred_role"]) if data.get("nationality"): queryset = queryset.filter(nationality=data["nationality"]) - if data.get("origin_competition"): - queryset = queryset.filter(origin_competition=data["origin_competition"]) - if data.get("origin_team"): - queryset = queryset.filter(origin_team=data["origin_team"]) queryset = _apply_min_max_filter(queryset, "height_min", "height_max", "height_cm", data) queryset = _apply_min_max_filter(queryset, "weight_min", "weight_max", "weight_kg", data) @@ -235,47 +223,62 @@ def annotate_player_metrics(queryset, data: dict | None = None): output_field=FloatField(), ), ), - default=Value(0.0), + default=Value(None), output_field=FloatField(), ) return queryset.annotate( - games_played_value=Coalesce( - Max("player_seasons__games_played", filter=context_filter), - Value(0, output_field=IntegerField()), + games_played_value=Max( + "player_seasons__games_played", + filter=context_filter, output_field=IntegerField(), ), - mpg_value=Coalesce(Max(mpg_expression, filter=context_filter), Value(0.0)), - ppg_value=Coalesce( - Max("player_seasons__stats__points", filter=context_filter), - Value(0, output_field=DecimalField(max_digits=6, decimal_places=2)), + mpg_value=Max(mpg_expression, filter=context_filter), + ppg_value=Max( + "player_seasons__stats__points", + filter=context_filter, output_field=DecimalField(max_digits=6, decimal_places=2), ), - rpg_value=Coalesce( - Max("player_seasons__stats__rebounds", filter=context_filter), - Value(0, output_field=DecimalField(max_digits=6, decimal_places=2)), + rpg_value=Max( + "player_seasons__stats__rebounds", + filter=context_filter, output_field=DecimalField(max_digits=6, decimal_places=2), ), - apg_value=Coalesce( - Max("player_seasons__stats__assists", filter=context_filter), - Value(0, output_field=DecimalField(max_digits=6, decimal_places=2)), + apg_value=Max( + "player_seasons__stats__assists", + filter=context_filter, output_field=DecimalField(max_digits=6, decimal_places=2), ), - spg_value=Coalesce( - Max("player_seasons__stats__steals", filter=context_filter), - Value(0, output_field=DecimalField(max_digits=6, decimal_places=2)), + spg_value=Max( + "player_seasons__stats__steals", + filter=context_filter, output_field=DecimalField(max_digits=6, decimal_places=2), ), - bpg_value=Coalesce( - Max("player_seasons__stats__blocks", filter=context_filter), - Value(0, output_field=DecimalField(max_digits=6, decimal_places=2)), + bpg_value=Max( + "player_seasons__stats__blocks", + filter=context_filter, output_field=DecimalField(max_digits=6, decimal_places=2), ), - top_efficiency=Coalesce( - Max("player_seasons__stats__player_efficiency_rating", filter=context_filter), - Value(0, output_field=DecimalField(max_digits=6, decimal_places=2)), + tov_value=Max( + "player_seasons__stats__turnovers", + filter=context_filter, output_field=DecimalField(max_digits=6, decimal_places=2), ), + fg_pct_value=Max( + "player_seasons__stats__fg_pct", + filter=context_filter, + output_field=DecimalField(max_digits=5, decimal_places=2), + ), + three_pct_value=Max( + "player_seasons__stats__three_pct", + filter=context_filter, + output_field=DecimalField(max_digits=5, decimal_places=2), + ), + ft_pct_value=Max( + "player_seasons__stats__ft_pct", + filter=context_filter, + output_field=DecimalField(max_digits=5, decimal_places=2), + ), ) diff --git a/apps/players/views.py b/apps/players/views.py index 993e87c..f22b082 100644 --- a/apps/players/views.py +++ b/apps/players/views.py @@ -7,7 +7,7 @@ from apps.scouting.models import FavoritePlayer from apps.stats.models import PlayerSeason from .forms import PlayerSearchForm -from .models import Player, PlayerCareerEntry +from .models import Player from .services.search import ( SEARCH_METRIC_SEMANTICS_TEXT, annotate_player_metrics, @@ -92,12 +92,6 @@ 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( @@ -108,9 +102,7 @@ class PlayerDetailView(DetailView): "origin_team", ) .prefetch_related( - "aliases", Prefetch("player_seasons", queryset=season_queryset), - Prefetch("career_entries", queryset=career_queryset), ) ) @@ -146,7 +138,6 @@ class PlayerDetailView(DetailView): context["age"] = calculate_age(player.birth_date) context["current_assignment"] = current_assignment - 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/templates/players/detail.html b/templates/players/detail.html index bbeac02..4cd6bbd 100644 --- a/templates/players/detail.html +++ b/templates/players/detail.html @@ -22,8 +22,6 @@

Summary

Nationality:
{{ player.nationality.name|default:"-" }}
-
Origin competition:
{{ player.origin_competition.name|default:"-" }}
-
Origin team:
{{ player.origin_team.name|default:"-" }}
Birth date:
{{ player.birth_date|date:"Y-m-d"|default:"-" }}
Age:
{{ age|default:"-" }}
Height:
{{ player.height_cm|default:"-" }} cm
@@ -47,14 +45,11 @@
-

Aliases

-
    - {% for alias in player.aliases.all %} -
  • {{ alias.alias }}{% if alias.source %} ({{ alias.source }}){% endif %}
    • - {% empty %} -
  • No aliases recorded.
    • - {% endfor %} -
+

Snapshot Coverage

+
+
Seasons imported:
{{ season_rows|length }}
+
Latest season:
{% if season_rows %}{{ season_rows.0.season.label|default:"-" }}{% else %}-{% endif %}
+
@@ -77,33 +72,6 @@ {% endif %} -
-

Career History

- {% if career_entries %} -
- - - - - - {% for entry in career_entries %} - - - - - - - - - {% endfor %} - -
SeasonTeamCompetitionRoleFromTo
{{ entry.season.label|default:"-" }}{{ entry.team.name|default:"-" }}{{ entry.competition.name|default:"-" }}{{ entry.role_snapshot.name|default:"-" }}{{ entry.start_date|date:"Y-m-d"|default:"-" }}{{ entry.end_date|date:"Y-m-d"|default:"-" }}
-
- {% else %} -
No career entries available.
- {% endif %} -
-

Season-by-Season Stats

{% if season_rows %} diff --git a/templates/players/index.html b/templates/players/index.html index 87d0044..fc47663 100644 --- a/templates/players/index.html +++ b/templates/players/index.html @@ -5,7 +5,7 @@ {% block content %}

Player Search

-

Filter players by profile, origin, context, and production metrics.

+

Filter players by profile, team-season context, and production metrics.

{% if search_has_errors %}

Please correct the highlighted filters.

@@ -56,8 +56,6 @@
{{ search_form.competition }}
{{ search_form.team }}
{{ search_form.season }}
-
{{ search_form.origin_competition }}
-
{{ search_form.origin_team }}
@@ -97,8 +95,6 @@
{{ search_form.three_pct_max }}
{{ search_form.ft_pct_min }}
{{ search_form.ft_pct_max }}
-
{{ search_form.efficiency_metric_min }}
-
{{ search_form.efficiency_metric_max }}
diff --git a/templates/players/partials/results.html b/templates/players/partials/results.html index ff25c65..cfdf430 100644 --- a/templates/players/partials/results.html +++ b/templates/players/partials/results.html @@ -36,13 +36,18 @@ Player Nationality Pos / Role - Origin Height / Weight Best Eligible Games Best Eligible MPG Best Eligible PPG Best Eligible RPG Best Eligible APG + Best Eligible SPG + Best Eligible BPG + Best Eligible TOV + Best Eligible FG% + Best Eligible 3P% + Best Eligible FT% {% if request.user.is_authenticated %}Watchlist{% endif %} @@ -52,16 +57,18 @@ {{ player.full_name }} {{ player.nationality.name|default:"-" }} {{ player.nominal_position.code|default:"-" }} / {{ player.inferred_role.name|default:"-" }} - - {{ player.origin_competition.name|default:"-" }} - {% if player.origin_team %}
{{ player.origin_team.name }}
{% endif %} - {{ player.height_cm|default:"-" }} / {{ player.weight_kg|default:"-" }} - {{ player.games_played_value|floatformat:0 }} - {{ player.mpg_value|floatformat:1 }} - {{ player.ppg_value|floatformat:1 }} - {{ player.rpg_value|floatformat:1 }} - {{ player.apg_value|floatformat:1 }} + {% if player.games_played_value is not None %}{{ player.games_played_value|floatformat:0 }}{% else %}-{% endif %} + {% if player.mpg_value is not None %}{{ player.mpg_value|floatformat:1 }}{% else %}-{% endif %} + {% if player.ppg_value is not None %}{{ player.ppg_value|floatformat:1 }}{% else %}-{% endif %} + {% if player.rpg_value is not None %}{{ player.rpg_value|floatformat:1 }}{% else %}-{% endif %} + {% if player.apg_value is not None %}{{ player.apg_value|floatformat:1 }}{% else %}-{% endif %} + {% if player.spg_value is not None %}{{ player.spg_value|floatformat:1 }}{% else %}-{% endif %} + {% if player.bpg_value is not None %}{{ player.bpg_value|floatformat:1 }}{% else %}-{% endif %} + {% if player.tov_value is not None %}{{ player.tov_value|floatformat:1 }}{% else %}-{% endif %} + {% if player.fg_pct_value is not None %}{{ player.fg_pct_value|floatformat:1 }}{% else %}-{% endif %} + {% if player.three_pct_value is not None %}{{ player.three_pct_value|floatformat:1 }}{% else %}-{% endif %} + {% if player.ft_pct_value is not None %}{{ player.ft_pct_value|floatformat:1 }}{% else %}-{% endif %} {% if request.user.is_authenticated %} {% if player.id in favorite_player_ids %} diff --git a/tests/test_api.py b/tests/test_api.py index 04a2be1..9117c6c 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -83,8 +83,6 @@ def test_players_api_search_consistent_with_ui_filters(client): nationality=nationality, nominal_position=position, inferred_role=role, - origin_competition=competition, - origin_team=team, ) ps = PlayerSeason.objects.create( player=matching, @@ -113,7 +111,7 @@ def test_players_api_search_consistent_with_ui_filters(client): ) params = { - "origin_competition": competition.id, + "competition": competition.id, "nominal_position": position.id, "points_per_game_min": "10", "sort": "ppg_desc", diff --git a/tests/test_players_filters_advanced.py b/tests/test_players_filters_advanced.py index b133790..dfa5f9d 100644 --- a/tests/test_players_filters_advanced.py +++ b/tests/test_players_filters_advanced.py @@ -110,8 +110,6 @@ def test_player_search_combined_filters_sorting_and_pagination(client): nationality=nationality, nominal_position=position, inferred_role=role, - origin_competition=competition, - origin_team=team, ) player_season = PlayerSeason.objects.create( player=player, @@ -135,7 +133,7 @@ def test_player_search_combined_filters_sorting_and_pagination(client): response = client.get( reverse("players:index"), data={ - "origin_competition": competition.id, + "competition": competition.id, "nominal_position": position.id, "sort": "ppg_desc", "page_size": 20, @@ -152,7 +150,7 @@ def test_player_search_combined_filters_sorting_and_pagination(client): page2 = client.get( reverse("players:index"), data={ - "origin_competition": competition.id, + "competition": competition.id, "nominal_position": position.id, "sort": "ppg_desc", "page_size": 20, diff --git a/tests/test_players_views.py b/tests/test_players_views.py index 7065000..96ef4ce 100644 --- a/tests/test_players_views.py +++ b/tests/test_players_views.py @@ -4,7 +4,7 @@ import pytest from django.urls import reverse from apps.competitions.models import Competition, Season -from apps.players.models import Nationality, Player, PlayerAlias, Position, Role +from apps.players.models import Nationality, Player, Position, Role from apps.stats.models import PlayerSeason, PlayerSeasonStats from apps.teams.models import Team @@ -127,14 +127,13 @@ def test_player_detail_page_loads(client): height_cm=201, weight_kg=95, ) - PlayerAlias.objects.create(player=player, alias="P. Martin") - response = client.get(reverse("players:detail", kwargs={"pk": player.pk})) assert response.status_code == 200 body = response.content.decode() assert "Paul Martin" in body - assert "P. Martin" in body + assert "Summary" in body + assert "Season-by-Season Stats" in body @pytest.mark.django_db @@ -242,3 +241,44 @@ def test_player_search_results_render_best_eligible_metric_labels(client): assert "Best Eligible PPG" in body assert "Best Eligible MPG" in body assert "best eligible values per player" in body.lower() + + +@pytest.mark.django_db +def test_player_search_results_render_dash_for_missing_eligible_metrics(client): + nationality = Nationality.objects.create(name="Norway", iso2_code="NO", iso3_code="NOR") + position = Position.objects.create(code="PF", name="Power Forward") + role = Role.objects.create(code="big", name="Big") + season = Season.objects.create(label="2025-2026", start_date=date(2025, 9, 1), end_date=date(2026, 6, 30)) + competition = Competition.objects.create( + name="BLNO", + slug="blno", + competition_type=Competition.CompetitionType.LEAGUE, + gender=Competition.Gender.MEN, + country=nationality, + ) + team = Team.objects.create(name="Oslo", slug="oslo", country=nationality) + + player = Player.objects.create( + first_name="Ole", + last_name="NoStats", + full_name="Ole NoStats", + birth_date=date(2001, 1, 1), + nationality=nationality, + nominal_position=position, + inferred_role=role, + ) + PlayerSeason.objects.create( + player=player, + season=season, + team=team, + competition=competition, + games_played=0, + minutes_played=0, + ) + + response = client.get(reverse("players:index"), data={"season": season.id}) + assert response.status_code == 200 + body = response.content.decode() + assert "Ole NoStats" in body + # Missing eligible values are rendered as '-' rather than misleading zeros. + assert body.count(">-") > 0