Make invalid search input explicit in UI and API

apps/api/views.py

@@ -1,5 +1,7 @@
 from rest_framework import generics
+from rest_framework import status
 from rest_framework.pagination import PageNumberPagination
+from rest_framework.response import Response
 from rest_framework.throttling import AnonRateThrottle, UserRateThrottle
 
 from apps.competitions.models import Competition, Season
@@ -39,17 +41,35 @@ class PlayerSearchApiView(ReadOnlyBaseAPIView, generics.ListAPIView):
     serializer_class = PlayerListSerializer
     pagination_class = ApiPagination
 
+    def get_search_form(self):
+        if not hasattr(self, "_search_form"):
+            self._search_form = PlayerSearchForm(self.request.query_params)
+        return self._search_form
+
+    def _validation_error_response(self):
+        form = self.get_search_form()
+        return Response(
+            {
+                "detail": "Invalid search parameters.",
+                "errors": form.errors.get_json_data(escape_html=True),
+            },
+            status=status.HTTP_400_BAD_REQUEST,
+        )
+
+    def list(self, request, *args, **kwargs):
+        form = self.get_search_form()
+        if form.is_bound and not form.is_valid():
+            return self._validation_error_response()
+        return super().list(request, *args, **kwargs)
+
     def get_queryset(self):
-        form = PlayerSearchForm(self.request.query_params or None)
+        form = self.get_search_form()
         queryset = base_player_queryset()
-        if form.is_valid():
-            queryset = filter_players(queryset, form.cleaned_data)
-            sort_key = form.cleaned_data.get("sort", "name_asc")
-            if sort_key in METRIC_SORT_KEYS:
-                queryset = annotate_player_metrics(queryset, form.cleaned_data)
-            queryset = apply_sorting(queryset, sort_key)
-        else:
-            queryset = queryset.order_by("full_name", "id")
+        queryset = filter_players(queryset, form.cleaned_data)
+        sort_key = form.cleaned_data.get("sort", "name_asc")
+        if sort_key in METRIC_SORT_KEYS:
+            queryset = annotate_player_metrics(queryset, form.cleaned_data)
+        queryset = apply_sorting(queryset, sort_key)
         return queryset
 
 

apps/players/views.py

@@ -33,7 +33,7 @@ class PlayerSearchView(ListView):
 
     def get_form(self):
         if not hasattr(self, "_search_form"):
-            self._search_form = PlayerSearchForm(self.request.GET or None)
+            self._search_form = PlayerSearchForm(self.request.GET)
         return self._search_form
 
     def get_paginate_by(self, queryset):
@@ -44,20 +44,23 @@ class PlayerSearchView(ListView):
 
     def get_queryset(self):
         form = self.get_form()
+        form_valid = form.is_valid()
+        if form.is_bound and not form_valid:
+            return Player.objects.none()
+
         queryset = base_player_queryset()
 
-        if form.is_valid():
-            queryset = filter_players(queryset, form.cleaned_data)
-            queryset = annotate_player_metrics(queryset, form.cleaned_data)
-            queryset = apply_sorting(queryset, form.cleaned_data.get("sort", "name_asc"))
-        else:
-            queryset = annotate_player_metrics(queryset).order_by("full_name", "id")
+        queryset = filter_players(queryset, form.cleaned_data)
+        queryset = annotate_player_metrics(queryset, form.cleaned_data)
+        queryset = apply_sorting(queryset, form.cleaned_data.get("sort", "name_asc"))
 
         return queryset
 
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
-        context["search_form"] = self.get_form()
+        search_form = self.get_form()
+        context["search_form"] = search_form
+        context["search_has_errors"] = search_form.is_bound and bool(search_form.errors)
         context["favorite_player_ids"] = set()
         if self.request.user.is_authenticated:
             player_ids = [player.id for player in context["players"]]

templates/players/index.html

@@ -6,6 +6,19 @@
 <section class="panel">
   <h1>Player Search</h1>
   <p class="mt-1 text-sm text-slate-600">Filter players by profile, origin, context, and production metrics.</p>
+  {% if search_has_errors %}
+    <div class="mt-4 rounded-md border border-rose-200 bg-rose-50 p-3 text-sm text-rose-800">
+      <p class="font-medium">Please correct the highlighted filters.</p>
+      {% for field in search_form %}
+        {% for error in field.errors %}
+          <p>{{ field.label }}: {{ error }}</p>
+        {% endfor %}
+      {% endfor %}
+      {% for error in search_form.non_field_errors %}
+        <p>{{ error }}</p>
+      {% endfor %}
+    </div>
+  {% endif %}
 
   <form
     method="get"

templates/players/partials/results.html

@@ -7,6 +7,20 @@
   </div>
 </div>
 
+{% if search_has_errors %}
+  <div class="mt-3 rounded-md border border-rose-200 bg-rose-50 p-3 text-sm text-rose-800">
+    <p class="font-medium">Search filters are invalid.</p>
+    {% for field in search_form %}
+      {% for error in field.errors %}
+        <p>{{ field.label }}: {{ error }}</p>
+      {% endfor %}
+    {% endfor %}
+    {% for error in search_form.non_field_errors %}
+      <p>{{ error }}</p>
+    {% endfor %}
+  </div>
+{% endif %}
+
 {% if request.user.is_authenticated %}
   {% include "scouting/partials/save_search_form.html" %}
 {% endif %}

tests/test_api.py

@@ -196,3 +196,28 @@ def test_api_combined_filters_respect_same_player_season_context(client):
     )
     assert response.status_code == 200
     assert response.json()["count"] == 0
+
+
+@pytest.mark.django_db
+def test_players_api_returns_400_for_invalid_numeric_filter(client):
+    response = client.get(reverse("api:players"), data={"points_per_game_min": "abc"})
+    assert response.status_code == 400
+    payload = response.json()
+    assert payload["detail"] == "Invalid search parameters."
+    assert "points_per_game_min" in payload["errors"]
+
+
+@pytest.mark.django_db
+def test_players_api_returns_400_for_invalid_choice_filter(client):
+    response = client.get(reverse("api:players"), data={"sort": "not-a-sort"})
+    assert response.status_code == 400
+    payload = response.json()
+    assert "sort" in payload["errors"]
+
+
+@pytest.mark.django_db
+def test_players_api_returns_400_for_invalid_range_combination(client):
+    response = client.get(reverse("api:players"), data={"age_min": 30, "age_max": 20})
+    assert response.status_code == 400
+    payload = response.json()
+    assert "age_max" in payload["errors"]

tests/test_players_views.py

@@ -135,3 +135,61 @@ def test_player_detail_page_loads(client):
     body = response.content.decode()
     assert "Paul Martin" in body
     assert "P. Martin" in body
+
+
+@pytest.mark.django_db
+def test_player_search_invalid_numeric_filter_shows_errors_and_no_broad_fallback(client):
+    nationality = Nationality.objects.create(name="Belgium", iso2_code="BE", iso3_code="BEL")
+    position = Position.objects.create(code="PG", name="Point Guard")
+    role = Role.objects.create(code="playmaker", name="Playmaker")
+    Player.objects.create(
+        first_name="Any",
+        last_name="Player",
+        full_name="Any Player",
+        birth_date=date(2000, 1, 1),
+        nationality=nationality,
+        nominal_position=position,
+        inferred_role=role,
+    )
+
+    response = client.get(reverse("players:index"), data={"points_per_game_min": "abc", "q": "Any"})
+
+    assert response.status_code == 200
+    assert list(response.context["players"]) == []
+    assert response.context["search_has_errors"] is True
+    assert "points per game min" in response.content.decode().lower()
+    assert response.context["search_form"]["q"].value() == "Any"
+
+
+@pytest.mark.django_db
+def test_player_search_invalid_choice_filter_shows_errors(client):
+    response = client.get(reverse("players:index"), data={"sort": "bad-sort"})
+    assert response.status_code == 200
+    assert list(response.context["players"]) == []
+    assert response.context["search_has_errors"] is True
+    assert "select a valid choice" in response.content.decode().lower()
+
+
+@pytest.mark.django_db
+def test_player_search_invalid_range_combination_shows_errors(client):
+    response = client.get(reverse("players:index"), data={"age_min": 30, "age_max": 20})
+    assert response.status_code == 200
+    assert list(response.context["players"]) == []
+    assert response.context["search_has_errors"] is True
+    body = response.content.decode().lower()
+    assert "age max" in body
+    assert "must be >=" in body or "must be >=" in body
+
+
+@pytest.mark.django_db
+def test_player_search_htmx_invalid_filters_return_validation_feedback(client):
+    response = client.get(
+        reverse("players:index"),
+        HTTP_HX_REQUEST="true",
+        data={"points_per_game_min": "abc"},
+    )
+
+    assert response.status_code == 200
+    body = response.content.decode().lower()
+    assert "search filters are invalid" in body
+    assert "points per game min" in body