From e6081428ae0f43fde18ef5e1bc0fcad014dc4d51 Mon Sep 17 00:00:00 2001 From: bisco Date: Sat, 11 Apr 2026 00:54:19 +0200 Subject: [PATCH] fix(ingestion): stop fabricating missing player positions --- README.md | 1 + app/scouting/importers/lba_public.py | 6 --- .../migrations/0010_alter_player_position.py | 18 ++++++++ app/scouting/models.py | 2 +- app/scouting/tests.py | 43 +++++++++++++++++++ 5 files changed, 63 insertions(+), 7 deletions(-) create mode 100644 app/scouting/migrations/0010_alter_player_position.py diff --git a/README.md b/README.md index 0ce6060..695baca 100644 --- a/README.md +++ b/README.md @@ -72,6 +72,7 @@ First public European importer (LBA Serie A scope): `docker compose --env-file .env -f infra/docker-compose.yml exec -T app python manage.py import_lba_public_serie_a --season 2025` - deterministic local-fixture import: `docker compose --env-file .env -f infra/docker-compose.yml exec -T app python manage.py import_lba_public_serie_a --season 2025 --fixture /app/scouting/sample_data/imports/lba_public_serie_a_fixture.json` +- note: when the public source does not expose a player position, HoopScout keeps `position` empty instead of fabricating a value. Legacy shared favorites and notes from the pre-auth MVP are cleared by the early-stage ownership migration so the app can move cleanly to user-scoped data. diff --git a/app/scouting/importers/lba_public.py b/app/scouting/importers/lba_public.py index 07ec3fc..63bb1dd 100644 --- a/app/scouting/importers/lba_public.py +++ b/app/scouting/importers/lba_public.py @@ -269,22 +269,16 @@ class LbaSerieAPublicImporter: player = Player.objects.filter(pk=mapping.object_id).first() if player is None: raise ImportValidationError("Player mapping points to a missing record.") - position = player.position or Player.Position.SG player.full_name = full_name player.first_name = record["name"] player.last_name = record["surname"] - player.position = position player.save() self.summary.players_updated += 1 else: - # LBA stats endpoint does not expose position directly. To satisfy the current - # required model field without guessing role/taxonomy data, we use a neutral - # default and keep role/specialty ownership untouched. player = Player.objects.create( full_name=full_name, first_name=record["name"], last_name=record["surname"], - position=Player.Position.SG, ) self.summary.players_created += 1 diff --git a/app/scouting/migrations/0010_alter_player_position.py b/app/scouting/migrations/0010_alter_player_position.py new file mode 100644 index 0000000..128a574 --- /dev/null +++ b/app/scouting/migrations/0010_alter_player_position.py @@ -0,0 +1,18 @@ +# Generated by Django 5.2.2 on 2026-04-10 22:53 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('scouting', '0009_externalentitymapping'), + ] + + operations = [ + migrations.AlterField( + model_name='player', + name='position', + field=models.CharField(blank=True, choices=[('PG', 'PG'), ('SG', 'SG'), ('SF', 'SF'), ('PF', 'PF'), ('C', 'C')], max_length=2, null=True), + ), + ] diff --git a/app/scouting/models.py b/app/scouting/models.py index ed78c91..f65f53b 100644 --- a/app/scouting/models.py +++ b/app/scouting/models.py @@ -42,7 +42,7 @@ class Player(models.Model): height_cm = models.DecimalField(max_digits=5, decimal_places=2, null=True, blank=True) weight_kg = models.DecimalField(max_digits=5, decimal_places=2, null=True, blank=True) wingspan_cm = models.DecimalField(max_digits=5, decimal_places=2, null=True, blank=True) - position = models.CharField(max_length=2, choices=Position.choices) + position = models.CharField(max_length=2, choices=Position.choices, null=True, blank=True) roles = models.ManyToManyField(Role, blank=True, related_name="players") specialties = models.ManyToManyField(Specialty, blank=True, related_name="players") created_at = models.DateTimeField(auto_now_add=True) diff --git a/app/scouting/tests.py b/app/scouting/tests.py index 4683940..ec78241 100644 --- a/app/scouting/tests.py +++ b/app/scouting/tests.py @@ -66,6 +66,14 @@ class ScoutingSearchViewsTests(TestCase): cls.player_wing.roles.add(cls.role_3d) cls.player_wing.specialties.add(cls.specialty_offball) + cls.player_unknown_position = Player.objects.create( + full_name="No Position Prospect", + birth_date=date(2001, 3, 3), + position=None, + height_cm=Decimal("180.00"), + weight_kg=Decimal("88.00"), + ) + cls.ctx_pg_good = PlayerSeason.objects.create( player=cls.player_pg, season=cls.season_2025, @@ -144,6 +152,24 @@ class ScoutingSearchViewsTests(TestCase): self.assertContains(response, self.player_pg.full_name) self.assertNotContains(response, self.player_wing.full_name) + def test_players_with_null_position_are_searchable_by_other_fields(self): + response = self.client.get( + reverse("scouting:player_list"), + {"name": "No Position"}, + ) + + self.assertEqual(response.status_code, 200) + self.assertContains(response, self.player_unknown_position.full_name) + + def test_position_filter_excludes_players_with_null_position(self): + response = self.client.get( + reverse("scouting:player_list"), + {"position": "SG"}, + ) + + self.assertEqual(response.status_code, 200) + self.assertNotContains(response, self.player_unknown_position.full_name) + def test_filter_by_wingspan_thresholds(self): response = self.client.get( reverse("scouting:player_list"), @@ -550,6 +576,23 @@ class FirstPublicEuropeanImporterTests(TestCase): self.assertTrue(PlayerSeason.objects.filter(player__full_name="Muhammad-Ali Abdur-Rahkman").exists()) self.assertTrue(PlayerSeasonStats.objects.filter(player_season__player__full_name="Muhammad-Ali Abdur-Rahkman").exists()) + def test_importer_does_not_assign_fake_position_when_source_position_is_missing(self): + self.run_import() + + player = Player.objects.get(full_name="Muhammad-Ali Abdur-Rahkman") + self.assertIsNone(player.position) + + def test_importer_preserves_existing_real_position_when_source_position_is_missing(self): + self.run_import() + player = Player.objects.get(full_name="Muhammad-Ali Abdur-Rahkman") + player.position = Player.Position.PG + player.save(update_fields=["position", "updated_at"]) + + self.run_import() + player.refresh_from_db() + + self.assertEqual(player.position, Player.Position.PG) + def test_importer_is_idempotent_for_same_input(self): self.run_import() first_counts = {