diff --git a/.env.example b/.env.example index e02aebe..4fb58bb 100644 --- a/.env.example +++ b/.env.example @@ -37,3 +37,7 @@ CELERY_TASK_TIME_LIMIT=1800 CELERY_TASK_SOFT_TIME_LIMIT=1500 API_THROTTLE_ANON=100/hour API_THROTTLE_USER=1000/hour + +# Testing (used with pytest-django) +# Keep development settings for local tests unless explicitly validating production settings. +PYTEST_ADDOPTS=-q diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 93ac88f..486126e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,46 +1,152 @@ # Contributing to HoopScout -## Branching Model (GitFlow Required) +## GitFlow Policy (Required) -- `main`: production-ready releases only -- `develop`: integration branch for completed features -- `feature/*`: feature development branches from `develop` +- `main`: production branch +- `develop`: integration branch +- `feature/*`: implementation branches from `develop` - `release/*`: release hardening branches from `develop` - `hotfix/*`: urgent production fixes from `main` -## Workflow +## Branch Naming Examples -1. Create branch from the correct base: - - `feature/*` from `develop` - - `release/*` from `develop` - - `hotfix/*` from `main` -2. Keep branch scope small and focused. -3. Open PR into the proper target branch. -4. Require passing CI checks and at least one review. -5. Squash or rebase merge according to repository policy. +Feature branches: -## Local Development +- `feature/player-search-performance` +- `feature/provider-sportradar-adapter` +- `feature/scouting-watchlist-notes` -1. `cp .env.example .env` -2. `docker compose up --build` -3. `docker compose exec web python manage.py migrate` -4. `docker compose exec web python manage.py createsuperuser` +Release branches: -## Testing +- `release/0.9.0-beta` +- `release/1.0.0` + +Hotfix branches: + +- `hotfix/fix-ingestion-run-crash` +- `hotfix/nginx-healthcheck-timeout` + +## Practical Workflow + +1. Sync base branch: ```bash -docker compose exec web pytest +git checkout develop +git pull ``` -## Commit Guidance +2. Create feature branch: -- Use clear commit messages with intent and scope. -- Avoid mixing refactors with feature behavior changes. -- Include migration files when model changes are introduced. +```bash +git checkout -b feature/your-feature-name +``` -## Definition of Done (MVP) +3. Implement and test in Docker. +4. Open PR into `develop`. +5. Require passing tests/checks and review before merge. -- Feature works in Dockerized environment. -- Tests added/updated for behavior change. -- Documentation updated when commands, config, or workflows change. -- No secrets committed. +For release: + +1. Create `release/*` from `develop`. +2. Stabilize, run regression tests, update docs/versioning. +3. Merge `release/*` into `main` and back into `develop`. + +For production urgent fix: + +1. Create `hotfix/*` from `main`. +2. Patch, test, merge to `main` and `develop`. + +## Local Development Setup + +```bash +cp .env.example .env +docker compose up --build +``` + +If needed: + +```bash +docker compose exec web python manage.py migrate +docker compose exec web python manage.py createsuperuser +``` + +## Testing Guidelines + +Run full suite: + +```bash +docker compose run --rm web sh -lc 'pip install -r requirements/dev.txt && pytest -q' +``` + +Run targeted modules while developing: + +```bash +docker compose run --rm web sh -lc 'pip install -r requirements/dev.txt && pytest -q tests/test_players_views.py' +``` + +## Migration Guidelines + +When schema changes are made: + +1. Create migrations. +2. Review migration SQL implications. +3. Commit migration files with model changes. + +Commands: + +```bash +docker compose exec web python manage.py makemigrations +docker compose exec web python manage.py migrate +``` + +## Ingestion Development Notes + +- Keep provider-specific code inside `apps/providers/*`. +- Keep orchestration and logging in `apps/ingestion/*`. +- Preserve idempotency (`update_or_create`, stable mappings). +- Store raw payloads only in designated diagnostic fields. + +## Suggested Milestones (GitFlow-Aligned) + +1. `M1 Infrastructure Foundation` + - Container/runtime hardening + - settings/security baseline + - CI test pipeline +2. `M2 Domain + Search Core` + - normalized schema + - HTMX search flow + - pagination/sorting/filter correctness +3. `M3 Scouting Productivity` + - saved searches/watchlist UX + - auth-protected workflows +4. `M4 Ingestion Reliability` + - provider adapters + - ingestion retries/rate-limit handling + - admin operations +5. `M5 Integration Surface` + - read-only API stabilization + - docs and onboarding hardening +6. `M6 Release Hardening` + - performance pass + - observability and failure drills + - release candidate QA + +## Recommended Feature Branch Development Order + +1. `feature/domain-stability-and-indexes` +2. `feature/search-query-optimization` +3. `feature/scouting-ux-polish` +4. `feature/provider-adapter-expansion` +5. `feature/ingestion-observability` +6. `feature/api-readonly-improvements` +7. `feature/security-and-rate-limit-tuning` + +This order prioritizes core data correctness first, then user value, then integration breadth. + +## Definition of Done + +- Runs correctly in Docker +- Tests added/updated for behavior changes +- Migrations included when schema changes occur +- Docs updated (`README`, `CONTRIBUTING`, `.env.example`) when workflows/config change +- No secrets committed diff --git a/README.md b/README.md index 762f986..bcaa0f0 100644 --- a/README.md +++ b/README.md @@ -1,112 +1,185 @@ # HoopScout -Production-minded basketball scouting and player search platform built with Django, HTMX, PostgreSQL, Redis, Celery, and nginx. +HoopScout is a production-minded basketball scouting and player search platform. +The main product experience is server-rendered Django Templates with HTMX enhancements. +A minimal read-only API is included as a secondary integration surface. -## Stack +## Core Stack - Python 3.12+ -- Django + Django Templates + HTMX (server-rendered) +- Django +- Django Templates + HTMX - PostgreSQL - Redis - Celery + Celery Beat -- nginx +- Django REST Framework (read-only API) +- pytest - Docker / Docker Compose +- nginx -## Project Structure +## Architecture Summary + +- Main UI: Django + HTMX (not SPA) +- Data layer: normalized domain models for players, seasons, competitions, teams, stats, scouting state +- Provider integration: adapter-based abstraction in `apps/providers` +- Ingestion orchestration: `apps/ingestion` with run/error logs and Celery task execution +- Optional API: read-only DRF endpoints under `/api/` + +## Repository Structure ```text . ├── apps/ -│ ├── core/ -│ ├── users/ -│ ├── players/ +│ ├── api/ │ ├── competitions/ -│ ├── teams/ -│ ├── stats/ -│ ├── scouting/ +│ ├── core/ +│ ├── ingestion/ +│ ├── players/ │ ├── providers/ -│ └── ingestion/ +│ ├── scouting/ +│ ├── stats/ +│ ├── teams/ +│ └── users/ ├── config/ │ └── settings/ ├── nginx/ ├── requirements/ ├── static/ ├── templates/ -└── tests/ +├── tests/ +├── docker-compose.yml +├── Dockerfile +└── entrypoint.sh ``` -## Setup +## Quick Start -1. Copy environment file: +1. Create local env file: ```bash cp .env.example .env ``` -2. Build and start services: +2. Build and run services: ```bash docker compose up --build ``` -3. Apply migrations (if auto-migrate disabled): +3. If `AUTO_APPLY_MIGRATIONS=0`, run migrations manually: ```bash docker compose exec web python manage.py migrate ``` -4. Create superuser: +4. Create a superuser: ```bash docker compose exec web python manage.py createsuperuser ``` -5. Access app: +5. Open the app: -- Application: http://localhost +- Web: http://localhost - Admin: http://localhost/admin/ -- Health endpoint: http://localhost/health/ +- Health: http://localhost/health/ +- API root endpoints: `/api/players/`, `/api/competitions/`, `/api/teams/`, `/api/seasons/` -## Authentication Routes +## Setup and Run Notes -- Signup: `/users/signup/` -- Login: `/users/login/` -- Logout: `/users/logout/` -- Dashboard (auth required): `/dashboard/` +- `web` service starts through `entrypoint.sh` and waits for PostgreSQL readiness. +- `celery_worker` executes background sync work. +- `celery_beat` supports scheduled jobs (future scheduling strategy can be added per provider). +- nginx proxies web traffic and serves static/media volume mounts. -## Tailwind Integration Strategy +## Docker Volumes and Persistence -Phase 2 keeps styling minimal and framework-neutral using `static/css/main.css`. -For upcoming phases, Tailwind will be integrated as a build step that emits compiled CSS into `static/css/` (e.g., via standalone Tailwind CLI or PostCSS in a dedicated frontend tooling container), while templates stay server-rendered. +`docker-compose.yml` uses named volumes: -## Development Commands +- `postgres_data`: PostgreSQL persistent database +- `static_data`: collected static assets +- `media_data`: user/provider media artifacts +- `runtime_data`: persistent runtime files (e.g., celery beat schedule, redis data) -Run tests: - -```bash -docker compose exec web pytest -``` - -Run Django shell: - -```bash -docker compose exec web python manage.py shell -``` +This keeps persistent state outside container lifecycles. ## Migrations -Create migration: +Create migration files: ```bash docker compose exec web python manage.py makemigrations ``` -Apply migration: +Apply migrations: ```bash docker compose exec web python manage.py migrate ``` -## GitFlow +## Testing -See [CONTRIBUTING.md](CONTRIBUTING.md) for branch model and PR workflow. +Run all tests: + +```bash +docker compose run --rm web sh -lc 'pip install -r requirements/dev.txt && pytest -q' +``` + +Run a focused module: + +```bash +docker compose run --rm web sh -lc 'pip install -r requirements/dev.txt && pytest -q tests/test_api.py' +``` + +## Superuser and Auth + +Create superuser: + +```bash +docker compose exec web python manage.py createsuperuser +``` + +Default auth routes: + +- Signup: `/users/signup/` +- Login: `/users/login/` +- Logout: `/users/logout/` + +## Ingestion and Manual Sync + +### Trigger via Django Admin + +- Open `/admin/` -> `IngestionRun` +- Use admin actions: + - `Queue full MVP sync` + - `Queue incremental MVP sync` + - `Retry selected ingestion runs` + +### Trigger from shell (manual) + +```bash +docker compose exec web python manage.py shell +``` + +```python +from apps.ingestion.tasks import trigger_full_sync +trigger_full_sync.delay(provider_namespace="mvp_demo") +``` + +### Logs and diagnostics + +- Run-level status/counters: `IngestionRun` +- Structured error records: `IngestionError` +- Provider entity mappings + diagnostic payload snippets: `ExternalMapping` + +## GitFlow Reference + +HoopScout uses strict GitFlow: + +- `main`: production-ready +- `develop`: integration +- `feature/*`: features from `develop` +- `release/*`: stabilization from `develop` +- `hotfix/*`: urgent production fixes from `main` + +See [CONTRIBUTING.md](CONTRIBUTING.md) for practical examples and milestone planning. diff --git a/tests/test_auth.py b/tests/test_auth.py index a5de28b..4a07182 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -21,3 +21,20 @@ def test_dashboard_requires_authentication(client): response = client.get(reverse("core:dashboard")) assert response.status_code == 302 assert reverse("users:login") in response.url + + +@pytest.mark.django_db +def test_login_logout_flow(client): + User.objects.create_user(username="login-user", email="login@example.com", password="StrongPass12345") + + login_response = client.post( + reverse("users:login"), + data={"username": "login-user", "password": "StrongPass12345"}, + follow=True, + ) + assert login_response.status_code == 200 + assert login_response.wsgi_request.user.is_authenticated + + logout_response = client.post(reverse("users:logout"), follow=True) + assert logout_response.status_code == 200 + assert not logout_response.wsgi_request.user.is_authenticated diff --git a/tests/test_ingestion_sync.py b/tests/test_ingestion_sync.py index 3b4bf15..1624cba 100644 --- a/tests/test_ingestion_sync.py +++ b/tests/test_ingestion_sync.py @@ -26,6 +26,47 @@ def test_run_full_sync_creates_domain_objects(settings): assert PlayerSeasonStats.objects.count() >= 1 +@pytest.mark.django_db +def test_full_sync_is_idempotent(settings): + settings.PROVIDER_DEFAULT_NAMESPACE = "mvp_demo" + + run_sync_job(provider_namespace="mvp_demo", job_type=IngestionRun.JobType.FULL_SYNC) + counts_after_first = { + "competition": Competition.objects.count(), + "team": Team.objects.count(), + "season": Season.objects.count(), + "player": Player.objects.count(), + "player_season": PlayerSeason.objects.count(), + "player_stats": PlayerSeasonStats.objects.count(), + } + + run_sync_job(provider_namespace="mvp_demo", job_type=IngestionRun.JobType.FULL_SYNC) + counts_after_second = { + "competition": Competition.objects.count(), + "team": Team.objects.count(), + "season": Season.objects.count(), + "player": Player.objects.count(), + "player_season": PlayerSeason.objects.count(), + "player_stats": PlayerSeasonStats.objects.count(), + } + + assert counts_after_first == counts_after_second + + +@pytest.mark.django_db +def test_incremental_sync_runs_successfully(settings): + settings.PROVIDER_DEFAULT_NAMESPACE = "mvp_demo" + + run = run_sync_job( + provider_namespace="mvp_demo", + job_type=IngestionRun.JobType.INCREMENTAL, + cursor="demo-cursor", + ) + + assert run.status == IngestionRun.RunStatus.SUCCESS + assert run.records_processed > 0 + + @pytest.mark.django_db def test_run_sync_handles_rate_limit(settings): settings.PROVIDER_DEFAULT_NAMESPACE = "mvp_demo" diff --git a/tests/test_integration_paths.py b/tests/test_integration_paths.py new file mode 100644 index 0000000..742dd00 --- /dev/null +++ b/tests/test_integration_paths.py @@ -0,0 +1,49 @@ +from datetime import date + +import pytest +from django.contrib.auth.models import User +from django.urls import reverse + +from apps.players.models import Nationality, Player, Position, Role +from apps.scouting.models import SavedSearch + + +@pytest.mark.django_db +def test_saved_search_run_filters_player_results(client): + user = User.objects.create_user(username="integration", password="pass12345") + client.force_login(user) + + nationality = Nationality.objects.create(name="Italy", iso2_code="IT", iso3_code="ITA") + position = Position.objects.create(code="PG", name="Point Guard") + role = Role.objects.create(code="playmaker", name="Playmaker") + + Player.objects.create( + first_name="Marco", + last_name="Rossi", + full_name="Marco Rossi", + birth_date=date(2001, 1, 1), + nationality=nationality, + nominal_position=position, + inferred_role=role, + ) + Player.objects.create( + first_name="Luca", + last_name="Bianchi", + full_name="Luca Bianchi", + birth_date=date(2001, 1, 1), + nationality=nationality, + nominal_position=position, + inferred_role=role, + ) + + saved = SavedSearch.objects.create( + user=user, + name="Only Marco", + filters={"q": "Marco", "sort": "name_asc"}, + ) + + response = client.get(reverse("scouting:saved_search_run", kwargs={"pk": saved.pk}), follow=True) + + assert response.status_code == 200 + assert "Marco Rossi" in response.content.decode() + assert "Luca Bianchi" not in response.content.decode() diff --git a/tests/test_models_domain.py b/tests/test_models_domain.py new file mode 100644 index 0000000..4ddfb98 --- /dev/null +++ b/tests/test_models_domain.py @@ -0,0 +1,92 @@ +from datetime import date + +import pytest +from django.contrib.auth.models import User +from django.db import IntegrityError + +from apps.competitions.models import Competition +from apps.players.models import Nationality, Player, Position, Role +from apps.providers.models import ExternalMapping +from apps.scouting.models import FavoritePlayer, SavedSearch + + +@pytest.mark.django_db +def test_player_unique_full_name_birth_date_constraint(): + nationality = Nationality.objects.create(name="Italy", iso2_code="IT", iso3_code="ITA") + position = Position.objects.create(code="PG", name="Point Guard") + role = Role.objects.create(code="playmaker", name="Playmaker") + + Player.objects.create( + first_name="Marco", + last_name="Rossi", + full_name="Marco Rossi", + birth_date=date(2001, 1, 1), + nationality=nationality, + nominal_position=position, + inferred_role=role, + ) + + with pytest.raises(IntegrityError): + Player.objects.create( + first_name="Marco", + last_name="Rossi", + full_name="Marco Rossi", + birth_date=date(2001, 1, 1), + nationality=nationality, + nominal_position=position, + inferred_role=role, + ) + + +@pytest.mark.django_db +def test_saved_search_unique_name_per_user_constraint(): + user = User.objects.create_user(username="u1", password="pass12345") + SavedSearch.objects.create(user=user, name="My Search", filters={"q": "rossi"}) + + with pytest.raises(IntegrityError): + SavedSearch.objects.create(user=user, name="My Search", filters={"q": "martin"}) + + +@pytest.mark.django_db +def test_favorite_unique_player_per_user_constraint(): + user = User.objects.create_user(username="u2", password="pass12345") + nationality = Nationality.objects.create(name="Spain", iso2_code="ES", iso3_code="ESP") + position = Position.objects.create(code="SF", name="Small Forward") + role = Role.objects.create(code="wing", name="Wing") + player = Player.objects.create( + first_name="Juan", + last_name="Perez", + full_name="Juan Perez", + birth_date=date(2000, 5, 1), + nationality=nationality, + nominal_position=position, + inferred_role=role, + ) + + FavoritePlayer.objects.create(user=user, player=player) + with pytest.raises(IntegrityError): + FavoritePlayer.objects.create(user=user, player=player) + + +@pytest.mark.django_db +def test_external_mapping_unique_provider_external_id_constraint(): + competition = Competition.objects.create( + name="Liga ACB", + slug="liga-acb", + competition_type=Competition.CompetitionType.LEAGUE, + gender=Competition.Gender.MEN, + level=1, + ) + + ExternalMapping.objects.create( + provider_namespace="mvp_demo", + external_id="comp-001", + content_object=competition, + ) + + with pytest.raises(IntegrityError): + ExternalMapping.objects.create( + provider_namespace="mvp_demo", + external_id="comp-001", + content_object=competition, + ) diff --git a/tests/test_players_filters_advanced.py b/tests/test_players_filters_advanced.py new file mode 100644 index 0000000..6a5c362 --- /dev/null +++ b/tests/test_players_filters_advanced.py @@ -0,0 +1,83 @@ +from datetime import date + +import pytest +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 + + +@pytest.mark.django_db +def test_player_search_stat_filter_and_sorting(client): + nationality = Nationality.objects.create(name="France", iso2_code="FR", iso3_code="FRA") + position = Position.objects.create(code="SG", name="Shooting Guard") + role = Role.objects.create(code="scorer", name="Scorer") + season = Season.objects.create(label="2025-2026", start_date=date(2025, 9, 1), end_date=date(2026, 6, 30)) + competition = Competition.objects.create( + name="LNB Pro A", + slug="lnb-pro-a", + competition_type=Competition.CompetitionType.LEAGUE, + gender=Competition.Gender.MEN, + country=nationality, + ) + team = Team.objects.create(name="Paris", slug="paris", country=nationality) + + p1 = Player.objects.create( + first_name="A", + last_name="One", + full_name="A One", + birth_date=date(2000, 1, 1), + nationality=nationality, + nominal_position=position, + inferred_role=role, + ) + p2 = Player.objects.create( + first_name="B", + last_name="Two", + full_name="B Two", + birth_date=date(2000, 1, 1), + nationality=nationality, + nominal_position=position, + inferred_role=role, + ) + + ps1 = PlayerSeason.objects.create(player=p1, season=season, team=team, competition=competition, games_played=20, minutes_played=500) + ps2 = PlayerSeason.objects.create(player=p2, season=season, team=team, competition=competition, games_played=20, minutes_played=700) + + PlayerSeasonStats.objects.create(player_season=ps1, points=10.0, rebounds=3, assists=2, steals=1, blocks=0.2, turnovers=1) + PlayerSeasonStats.objects.create(player_season=ps2, points=19.0, rebounds=4, assists=5, steals=1.5, blocks=0.4, turnovers=2) + + response = client.get( + reverse("players:index"), + data={"points_per_game_min": "15", "sort": "ppg_desc"}, + ) + + assert response.status_code == 200 + players = list(response.context["players"]) + assert len(players) == 1 + assert players[0].full_name == "B Two" + + +@pytest.mark.django_db +def test_player_search_pagination_preserves_querystring(client): + nationality = Nationality.objects.create(name="Germany", iso2_code="DE", iso3_code="DEU") + position = Position.objects.create(code="PF", name="Power Forward") + role = Role.objects.create(code="big", name="Big") + + for idx in range(25): + Player.objects.create( + first_name=f"F{idx}", + last_name=f"L{idx}", + full_name=f"Player {idx}", + birth_date=date(2000, 1, 1), + nationality=nationality, + nominal_position=position, + inferred_role=role, + ) + + response = client.get(reverse("players:index"), data={"q": "Player", "page_size": 20, "page": 2}) + + assert response.status_code == 200 + assert response.context["page_obj"].number == 2 diff --git a/tests/test_provider_adapter.py b/tests/test_provider_adapter.py new file mode 100644 index 0000000..6d5f5a3 --- /dev/null +++ b/tests/test_provider_adapter.py @@ -0,0 +1,43 @@ +import os + +import pytest + +from apps.providers.adapters.mvp_provider import MvpDemoProviderAdapter +from apps.providers.exceptions import ProviderNotFoundError, ProviderRateLimitError +from apps.providers.registry import get_provider + + +@pytest.mark.django_db +def test_mvp_provider_fetch_and_search_players(): + adapter = MvpDemoProviderAdapter() + + players = adapter.fetch_players() + assert len(players) >= 2 + + results = adapter.search_players(query="luca") + assert any("Luca" in item["full_name"] for item in results) + + detail = adapter.fetch_player(external_player_id="player-001") + assert detail is not None + assert detail["full_name"] == "Luca Rinaldi" + + +@pytest.mark.django_db +def test_mvp_provider_rate_limit_signal(): + os.environ["PROVIDER_MVP_FORCE_RATE_LIMIT"] = "1" + adapter = MvpDemoProviderAdapter() + + with pytest.raises(ProviderRateLimitError): + adapter.fetch_players() + + os.environ.pop("PROVIDER_MVP_FORCE_RATE_LIMIT", None) + + +@pytest.mark.django_db +def test_provider_registry_resolution(settings): + settings.PROVIDER_DEFAULT_NAMESPACE = "mvp_demo" + provider = get_provider() + assert isinstance(provider, MvpDemoProviderAdapter) + + with pytest.raises(ProviderNotFoundError): + get_provider("does-not-exist") diff --git a/tests/test_scouting_views.py b/tests/test_scouting_views.py index e7de8aa..c84adec 100644 --- a/tests/test_scouting_views.py +++ b/tests/test_scouting_views.py @@ -85,3 +85,46 @@ def test_favorite_toggle_adds_and_removes(client): remove_resp = client.post(reverse("scouting:favorite_toggle", kwargs={"player_id": player.id})) assert remove_resp.status_code == 302 assert not FavoritePlayer.objects.filter(user=user, player=player).exists() + + +@pytest.mark.django_db +def test_favorite_toggle_htmx_returns_partial_button(client): + user = User.objects.create_user(username="scout4", password="pass12345") + client.force_login(user) + + nationality = Nationality.objects.create(name="France", iso2_code="FR", iso3_code="FRA") + position = Position.objects.create(code="PF", name="Power Forward") + role = Role.objects.create(code="big", name="Big") + player = Player.objects.create( + first_name="Pierre", + last_name="Durand", + full_name="Pierre Durand", + birth_date=date(2001, 3, 3), + nationality=nationality, + nominal_position=position, + inferred_role=role, + ) + + response = client.post( + reverse("scouting:favorite_toggle", kwargs={"player_id": player.id}), + HTTP_HX_REQUEST="true", + data={"next": reverse("players:index")}, + ) + + assert response.status_code == 200 + assert "Remove favorite" in response.content.decode() + + +@pytest.mark.django_db +def test_save_search_htmx_feedback(client): + user = User.objects.create_user(username="scout5", password="pass12345") + client.force_login(user) + + response = client.post( + reverse("scouting:saved_search_create"), + HTTP_HX_REQUEST="true", + data={"name": "HTMX Search", "q": "john", "sort": "name_asc"}, + ) + + assert response.status_code == 200 + assert "created" in response.content.decode().lower()