feat: add user-scoped favorites and notes

This commit is contained in:
bisco
2026-04-07 18:11:19 +02:00
parent a5e1d841df
commit caa1f8354d
12 changed files with 364 additions and 93 deletions

View File

@ -10,8 +10,9 @@ The current application baseline provides:
- player scouting search with player, context, and stat filters - player scouting search with player, context, and stat filters
- matching season/team/competition context on search results - matching season/team/competition context on search results
- result sorting and pagination - result sorting and pagination
- a shared development shortlist for favorite players - login/logout with Django built-in authentication
- shared plain-text scouting notes on player detail pages - user-scoped shortlist favorites
- user-scoped plain-text scouting notes on player detail pages
Accepted technical and product-shaping decisions live in: Accepted technical and product-shaping decisions live in:
- `docs/ARCHITECTURE.md` - `docs/ARCHITECTURE.md`
@ -48,9 +49,12 @@ Accepted technical and product-shaping decisions live in:
1. Start the stack with `docker compose --env-file .env -f infra/docker-compose.yml up -d --build`. 1. Start the stack with `docker compose --env-file .env -f infra/docker-compose.yml up -d --build`.
2. Apply migrations with `docker compose --env-file .env -f infra/docker-compose.yml exec -T app python manage.py migrate`. 2. Apply migrations with `docker compose --env-file .env -f infra/docker-compose.yml exec -T app python manage.py migrate`.
3. Load sample data with `docker compose --env-file .env -f infra/docker-compose.yml exec -T app python manage.py seed_scouting_data`. 3. Load sample data with `docker compose --env-file .env -f infra/docker-compose.yml exec -T app python manage.py seed_scouting_data`.
4. Visit `http://127.0.0.1:8000/players/` to explore the scouting search MVP. 4. Create a local user with `docker compose --env-file .env -f infra/docker-compose.yml exec -T app python manage.py createsuperuser` if you need a development login.
5. Use player detail pages to manage shortlist entries and shared scouting notes. 5. Visit `http://127.0.0.1:8000/players/` to explore the scouting search MVP.
6. Use `http://127.0.0.1:8000/favorites/` to review the shared development shortlist. 6. Log in at `http://127.0.0.1:8000/accounts/login/` to manage your own shortlist and notes.
7. Use `http://127.0.0.1:8000/favorites/` to review your user-scoped shortlist.
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.
## Workflow ## Workflow

View File

@ -66,3 +66,7 @@ USE_TZ = True
STATIC_URL = "static/" STATIC_URL = "static/"
DEFAULT_AUTO_FIELD = "django.db.models.BigAutoField" DEFAULT_AUTO_FIELD = "django.db.models.BigAutoField"
LOGIN_URL = "login"
LOGIN_REDIRECT_URL = "scouting:player_list"
LOGOUT_REDIRECT_URL = "scouting:player_list"

View File

@ -1,7 +1,10 @@
from django.contrib import admin from django.contrib import admin
from django.contrib.auth import views as auth_views
from django.urls import include, path from django.urls import include, path
urlpatterns = [ urlpatterns = [
path("admin/", admin.site.urls), path("admin/", admin.site.urls),
path("accounts/login/", auth_views.LoginView.as_view(template_name="registration/login.html"), name="login"),
path("accounts/logout/", auth_views.LogoutView.as_view(), name="logout"),
path("", include("scouting.urls")), path("", include("scouting.urls")),
] ]

View File

@ -74,11 +74,11 @@ class PlayerSeasonStatsAdmin(admin.ModelAdmin):
@admin.register(FavoritePlayer) @admin.register(FavoritePlayer)
class FavoritePlayerAdmin(admin.ModelAdmin): class FavoritePlayerAdmin(admin.ModelAdmin):
list_display = ("player", "created_at") list_display = ("user", "player", "created_at")
search_fields = ("player__full_name",) search_fields = ("user__username", "player__full_name")
@admin.register(PlayerNote) @admin.register(PlayerNote)
class PlayerNoteAdmin(admin.ModelAdmin): class PlayerNoteAdmin(admin.ModelAdmin):
list_display = ("player", "created_at", "updated_at") list_display = ("user", "player", "created_at", "updated_at")
search_fields = ("player__full_name", "body") search_fields = ("user__username", "player__full_name", "body")

View File

@ -0,0 +1,75 @@
from django.conf import settings
from django.db import migrations, models
def clear_shared_shortlist_and_notes(apps, schema_editor):
FavoritePlayer = apps.get_model("scouting", "FavoritePlayer")
PlayerNote = apps.get_model("scouting", "PlayerNote")
FavoritePlayer.objects.all().delete()
PlayerNote.objects.all().delete()
class Migration(migrations.Migration):
dependencies = [
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
("scouting", "0006_playernote"),
]
operations = [
migrations.AddField(
model_name="favoriteplayer",
name="user",
field=models.ForeignKey(
null=True,
on_delete=models.CASCADE,
related_name="favorite_players",
to=settings.AUTH_USER_MODEL,
),
),
migrations.AddField(
model_name="playernote",
name="user",
field=models.ForeignKey(
null=True,
on_delete=models.CASCADE,
related_name="player_notes",
to=settings.AUTH_USER_MODEL,
),
),
migrations.RunPython(clear_shared_shortlist_and_notes, migrations.RunPython.noop),
migrations.RemoveField(
model_name="favoriteplayer",
name="player",
),
migrations.AddField(
model_name="favoriteplayer",
name="player",
field=models.ForeignKey(
on_delete=models.CASCADE,
related_name="favorite_entries",
to="scouting.player",
),
),
migrations.AlterField(
model_name="favoriteplayer",
name="user",
field=models.ForeignKey(
on_delete=models.CASCADE,
related_name="favorite_players",
to=settings.AUTH_USER_MODEL,
),
),
migrations.AlterField(
model_name="playernote",
name="user",
field=models.ForeignKey(
on_delete=models.CASCADE,
related_name="player_notes",
to=settings.AUTH_USER_MODEL,
),
),
migrations.AddConstraint(
model_name="favoriteplayer",
constraint=models.UniqueConstraint(fields=("user", "player"), name="uniq_favorite_player_per_user"),
),
]

View File

@ -1,3 +1,4 @@
from django.conf import settings
from django.db import models from django.db import models
@ -164,25 +165,34 @@ class PlayerSeasonStats(models.Model):
class FavoritePlayer(models.Model): class FavoritePlayer(models.Model):
# Phase-2 MVP uses a single shared development shortlist instead of user-scoped user = models.ForeignKey(
# favorites so the workflow stays useful without introducing auth complexity yet. settings.AUTH_USER_MODEL,
player = models.OneToOneField( on_delete=models.CASCADE,
related_name="favorite_players",
)
player = models.ForeignKey(
Player, Player,
on_delete=models.CASCADE, on_delete=models.CASCADE,
related_name="favorite_entry", related_name="favorite_entries",
) )
created_at = models.DateTimeField(auto_now_add=True) created_at = models.DateTimeField(auto_now_add=True)
class Meta: class Meta:
ordering = ["-created_at", "player__full_name"] ordering = ["-created_at", "player__full_name"]
constraints = [
models.UniqueConstraint(fields=["user", "player"], name="uniq_favorite_player_per_user"),
]
def __str__(self) -> str: def __str__(self) -> str:
return f"Favorite: {self.player.full_name}" return f"Favorite: {self.user} -> {self.player.full_name}"
class PlayerNote(models.Model): class PlayerNote(models.Model):
# Phase-2 MVP keeps notes shared within the local development environment so user = models.ForeignKey(
# scouting observations can be used immediately without introducing auth yet. settings.AUTH_USER_MODEL,
on_delete=models.CASCADE,
related_name="player_notes",
)
player = models.ForeignKey( player = models.ForeignKey(
Player, Player,
on_delete=models.CASCADE, on_delete=models.CASCADE,
@ -196,4 +206,4 @@ class PlayerNote(models.Model):
ordering = ["-created_at", "-id"] ordering = ["-created_at", "-id"]
def __str__(self) -> str: def __str__(self) -> str:
return f"Note for {self.player.full_name}" return f"Note by {self.user} for {self.player.full_name}"

View File

@ -0,0 +1,24 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Log In</title>
</head>
<body>
<p><a href="{% url 'scouting:player_list' %}">Back to search</a></p>
<h1>Log In</h1>
{% if form.errors %}
<p>Your username and password did not match. Please try again.</p>
{% endif %}
<form method="post">
{% csrf_token %}
{{ form.as_p }}
{% if next %}
<input type="hidden" name="next" value="{{ next }}">
{% endif %}
<button type="submit">Log in</button>
</form>
</body>
</html>

View File

@ -7,9 +7,14 @@
<body> <body>
<p> <p>
<a href="{% url 'scouting:player_list' %}">Back to search</a> <a href="{% url 'scouting:player_list' %}">Back to search</a>
| Signed in as {{ request.user.username }}
| <form method="post" action="{% url 'logout' %}" style="display:inline;">
{% csrf_token %}
<button type="submit">Log out</button>
</form>
</p> </p>
<h1>Shared Development Shortlist</h1> <h1>Your Shortlist</h1>
<p>This MVP shortlist is shared across the local development environment.</p> <p>This page shows favorites saved only for your account.</p>
<ul> <ul>
{% for entry in favorites %} {% for entry in favorites %}

View File

@ -7,12 +7,22 @@
<body> <body>
<p> <p>
<a href="{% url 'scouting:player_list' %}">Back to search</a> <a href="{% url 'scouting:player_list' %}">Back to search</a>
{% if request.user.is_authenticated %}
| <a href="{% url 'scouting:favorites_list' %}">View shortlist</a> | <a href="{% url 'scouting:favorites_list' %}">View shortlist</a>
| Signed in as {{ request.user.username }}
| <form method="post" action="{% url 'logout' %}" style="display:inline;">
{% csrf_token %}
<button type="submit">Log out</button>
</form>
{% else %}
| <a href="{% url 'login' %}?next={{ request.get_full_path|urlencode }}">Log in</a>
{% endif %}
</p> </p>
<h1>{{ player.full_name }}</h1> <h1>{{ player.full_name }}</h1>
{% if request.user.is_authenticated %}
{% if is_favorite %} {% if is_favorite %}
<p><strong>On the shared development shortlist.</strong></p> <p><strong>On your shortlist.</strong></p>
<form method="post" action="{% url 'scouting:remove_favorite' player.id %}"> <form method="post" action="{% url 'scouting:remove_favorite' player.id %}">
{% csrf_token %} {% csrf_token %}
<input type="hidden" name="next" value="{{ request.get_full_path }}"> <input type="hidden" name="next" value="{{ request.get_full_path }}">
@ -25,6 +35,9 @@
<button type="submit">Add to shortlist</button> <button type="submit">Add to shortlist</button>
</form> </form>
{% endif %} {% endif %}
{% else %}
<p><a href="{% url 'login' %}?next={{ request.get_full_path|urlencode }}">Log in to manage your shortlist</a></p>
{% endif %}
<p>Position: {{ player.position }}</p> <p>Position: {{ player.position }}</p>
<p>Nationality: {{ player.nationality|default:"-" }}</p> <p>Nationality: {{ player.nationality|default:"-" }}</p>
@ -52,7 +65,8 @@
</p> </p>
<h2>Scouting Notes</h2> <h2>Scouting Notes</h2>
<p>Notes are shared across the local development shortlist workflow in this MVP.</p> {% if request.user.is_authenticated %}
<p>These notes are visible only to your account.</p>
<form method="post" action="{% url 'scouting:add_note' player.id %}"> <form method="post" action="{% url 'scouting:add_note' player.id %}">
{% csrf_token %} {% csrf_token %}
<input type="hidden" name="next" value="{{ request.get_full_path }}"> <input type="hidden" name="next" value="{{ request.get_full_path }}">
@ -60,17 +74,22 @@
<textarea id="id_body" name="body" rows="4" cols="60"></textarea> <textarea id="id_body" name="body" rows="4" cols="60"></textarea>
<button type="submit">Save note</button> <button type="submit">Save note</button>
</form> </form>
{% else %}
<p><a href="{% url 'login' %}?next={{ request.get_full_path|urlencode }}">Log in to add personal notes</a></p>
{% endif %}
<ul> <ul>
{% for note in notes %} {% for note in notes %}
<li> <li>
<div>{{ note.body|linebreaksbr }}</div> <div>{{ note.body|linebreaksbr }}</div>
<small>Created: {{ note.created_at }}</small> <small>Created: {{ note.created_at }}</small>
{% if request.user.is_authenticated %}
<form method="post" action="{% url 'scouting:delete_note' player.id note.id %}"> <form method="post" action="{% url 'scouting:delete_note' player.id note.id %}">
{% csrf_token %} {% csrf_token %}
<input type="hidden" name="next" value="{{ request.get_full_path }}"> <input type="hidden" name="next" value="{{ request.get_full_path }}">
<button type="submit">Delete note</button> <button type="submit">Delete note</button>
</form> </form>
{% endif %}
</li> </li>
{% empty %} {% empty %}
<li>No notes yet.</li> <li>No notes yet.</li>

View File

@ -6,7 +6,18 @@
</head> </head>
<body> <body>
<h1>Scout Search</h1> <h1>Scout Search</h1>
<p><a href="{% url 'scouting:favorites_list' %}">View shortlist</a></p> <p>
{% if request.user.is_authenticated %}
Signed in as {{ request.user.username }} |
<a href="{% url 'scouting:favorites_list' %}">View shortlist</a> |
<form method="post" action="{% url 'logout' %}" style="display:inline;">
{% csrf_token %}
<button type="submit">Log out</button>
</form>
{% else %}
<a href="{% url 'login' %}?next={{ request.get_full_path|urlencode }}">Log in</a>
{% endif %}
</p>
<form method="get"> <form method="get">
<fieldset> <fieldset>
@ -65,6 +76,7 @@
<li> <li>
<a href="{% url 'scouting:player_detail' player.id %}">{{ player.full_name }}</a> <a href="{% url 'scouting:player_detail' player.id %}">{{ player.full_name }}</a>
({{ player.position }}) ({{ player.position }})
{% if request.user.is_authenticated %}
{% if player.is_favorite %} {% if player.is_favorite %}
<strong>Shortlisted</strong> <strong>Shortlisted</strong>
<form method="post" action="{% url 'scouting:remove_favorite' player.id %}"> <form method="post" action="{% url 'scouting:remove_favorite' player.id %}">
@ -79,6 +91,9 @@
<button type="submit">Add to shortlist</button> <button type="submit">Add to shortlist</button>
</form> </form>
{% endif %} {% endif %}
{% else %}
<a href="{% url 'login' %}?next={{ request.get_full_path|urlencode }}">Log in to shortlist</a>
{% endif %}
{% if player.matching_context %} {% if player.matching_context %}
<div> <div>
Match context: Match context:

View File

@ -1,12 +1,17 @@
from datetime import date from datetime import date
from decimal import Decimal from decimal import Decimal
from django.contrib.auth import get_user_model
from django.core.management import call_command from django.core.management import call_command
from django.test import TestCase from django.db import connection
from django.db.migrations.executor import MigrationExecutor
from django.test import TestCase, TransactionTestCase
from django.urls import reverse from django.urls import reverse
from .models import Competition, FavoritePlayer, Player, PlayerNote, PlayerSeason, PlayerSeasonStats, Role, Season, Specialty, Team from .models import Competition, FavoritePlayer, Player, PlayerNote, PlayerSeason, PlayerSeasonStats, Role, Season, Specialty, Team
User = get_user_model()
class ScoutingSearchViewsTests(TestCase): class ScoutingSearchViewsTests(TestCase):
@classmethod @classmethod
@ -416,6 +421,8 @@ class SeedScoutingDataCommandTests(TestCase):
class FavoritePlayerViewsTests(TestCase): class FavoritePlayerViewsTests(TestCase):
@classmethod @classmethod
def setUpTestData(cls): def setUpTestData(cls):
cls.user = User.objects.create_user(username="scout_a", password="pass12345")
cls.other_user = User.objects.create_user(username="scout_b", password="pass12345")
cls.player = Player.objects.create( cls.player = Player.objects.create(
full_name="Favorite Prospect", full_name="Favorite Prospect",
birth_date=date(2001, 4, 4), birth_date=date(2001, 4, 4),
@ -431,17 +438,31 @@ class FavoritePlayerViewsTests(TestCase):
weight_kg=Decimal("94.00"), weight_kg=Decimal("94.00"),
) )
def test_adding_player_to_favorites(self): def test_login_page_loads(self):
response = self.client.get(reverse("login"))
self.assertEqual(response.status_code, 200)
self.assertContains(response, "Log In")
def test_login_works_for_existing_user(self):
response = self.client.post(
reverse("login"),
{"username": "scout_a", "password": "pass12345"},
)
self.assertEqual(response.status_code, 302)
def test_authenticated_user_can_add_player_to_favorites(self):
self.client.force_login(self.user)
response = self.client.post( response = self.client.post(
reverse("scouting:add_favorite", args=[self.player.id]), reverse("scouting:add_favorite", args=[self.player.id]),
{"next": reverse("scouting:player_detail", args=[self.player.id])}, {"next": reverse("scouting:player_detail", args=[self.player.id])},
) )
self.assertRedirects(response, reverse("scouting:player_detail", args=[self.player.id])) self.assertRedirects(response, reverse("scouting:player_detail", args=[self.player.id]))
self.assertTrue(FavoritePlayer.objects.filter(player=self.player).exists()) self.assertTrue(FavoritePlayer.objects.filter(user=self.user, player=self.player).exists())
def test_removing_player_from_favorites(self): def test_authenticated_user_can_remove_player_from_favorites(self):
FavoritePlayer.objects.create(player=self.player) FavoritePlayer.objects.create(user=self.user, player=self.player)
self.client.force_login(self.user)
response = self.client.post( response = self.client.post(
reverse("scouting:remove_favorite", args=[self.player.id]), reverse("scouting:remove_favorite", args=[self.player.id]),
@ -449,37 +470,55 @@ class FavoritePlayerViewsTests(TestCase):
) )
self.assertRedirects(response, reverse("scouting:favorites_list")) self.assertRedirects(response, reverse("scouting:favorites_list"))
self.assertFalse(FavoritePlayer.objects.filter(player=self.player).exists()) self.assertFalse(FavoritePlayer.objects.filter(user=self.user, player=self.player).exists())
def test_favorites_list_page_loads(self): def test_unauthenticated_user_cannot_add_or_remove_favorites(self):
FavoritePlayer.objects.create(player=self.player) add_response = self.client.post(reverse("scouting:add_favorite", args=[self.player.id]))
self.assertRedirects(add_response, f"{reverse('login')}?next={reverse('scouting:add_favorite', args=[self.player.id])}")
FavoritePlayer.objects.create(user=self.user, player=self.player)
remove_response = self.client.post(reverse("scouting:remove_favorite", args=[self.player.id]))
self.assertRedirects(remove_response, f"{reverse('login')}?next={reverse('scouting:remove_favorite', args=[self.player.id])}")
def test_favorites_page_is_user_scoped(self):
FavoritePlayer.objects.create(user=self.user, player=self.player)
FavoritePlayer.objects.create(user=self.other_user, player=self.other_player)
self.client.force_login(self.user)
response = self.client.get(reverse("scouting:favorites_list")) response = self.client.get(reverse("scouting:favorites_list"))
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
self.assertContains(response, "Favorite Prospect") self.assertContains(response, "Favorite Prospect")
self.assertNotContains(response, "Other Prospect")
def test_favorite_state_is_visible_on_detail_and_search_pages(self): def test_favorite_state_is_visible_on_detail_and_search_pages_for_logged_in_user(self):
FavoritePlayer.objects.create(player=self.player) FavoritePlayer.objects.create(user=self.user, player=self.player)
self.client.force_login(self.user)
detail_response = self.client.get(reverse("scouting:player_detail", args=[self.player.id])) detail_response = self.client.get(reverse("scouting:player_detail", args=[self.player.id]))
list_response = self.client.get(reverse("scouting:player_list")) list_response = self.client.get(reverse("scouting:player_list"))
self.assertContains(detail_response, "On the shared development shortlist.") self.assertContains(detail_response, "On your shortlist.")
self.assertContains(detail_response, "Remove from shortlist") self.assertContains(detail_response, "Remove from shortlist")
self.assertContains(list_response, "Shortlisted") self.assertContains(list_response, "Shortlisted")
def test_search_and_detail_pages_still_load_after_favorites_integration(self): def test_search_and_detail_pages_still_load_after_user_scoping(self):
search_response = self.client.get(reverse("scouting:player_list")) search_response = self.client.get(reverse("scouting:player_list"))
detail_response = self.client.get(reverse("scouting:player_detail", args=[self.player.id])) detail_response = self.client.get(reverse("scouting:player_detail", args=[self.player.id]))
self.assertEqual(search_response.status_code, 200) self.assertEqual(search_response.status_code, 200)
self.assertEqual(detail_response.status_code, 200) self.assertEqual(detail_response.status_code, 200)
def test_favorites_page_requires_login(self):
response = self.client.get(reverse("scouting:favorites_list"))
self.assertRedirects(response, f"{reverse('login')}?next={reverse('scouting:favorites_list')}")
class PlayerNoteViewsTests(TestCase): class PlayerNoteViewsTests(TestCase):
@classmethod @classmethod
def setUpTestData(cls): def setUpTestData(cls):
cls.user = User.objects.create_user(username="note_owner", password="pass12345")
cls.other_user = User.objects.create_user(username="note_other", password="pass12345")
cls.player = Player.objects.create( cls.player = Player.objects.create(
full_name="Notes Prospect", full_name="Notes Prospect",
birth_date=date(2001, 8, 8), birth_date=date(2001, 8, 8),
@ -488,7 +527,8 @@ class PlayerNoteViewsTests(TestCase):
weight_kg=Decimal("97.00"), weight_kg=Decimal("97.00"),
) )
def test_adding_note_to_player(self): def test_authenticated_user_can_add_note_to_player(self):
self.client.force_login(self.user)
response = self.client.post( response = self.client.post(
reverse("scouting:add_note", args=[self.player.id]), reverse("scouting:add_note", args=[self.player.id]),
{ {
@ -498,11 +538,16 @@ class PlayerNoteViewsTests(TestCase):
) )
self.assertRedirects(response, reverse("scouting:player_detail", args=[self.player.id])) self.assertRedirects(response, reverse("scouting:player_detail", args=[self.player.id]))
note = PlayerNote.objects.get(player=self.player) note = PlayerNote.objects.get(player=self.player, user=self.user)
self.assertEqual(note.body, "Shows good weak-side help instincts.") self.assertEqual(note.body, "Shows good weak-side help instincts.")
def test_deleting_note(self): def test_authenticated_user_can_delete_note(self):
note = PlayerNote.objects.create(player=self.player, body="Needs tighter handle under pressure.") note = PlayerNote.objects.create(
user=self.user,
player=self.player,
body="Needs tighter handle under pressure.",
)
self.client.force_login(self.user)
response = self.client.post( response = self.client.post(
reverse("scouting:delete_note", args=[self.player.id, note.id]), reverse("scouting:delete_note", args=[self.player.id, note.id]),
@ -512,17 +557,35 @@ class PlayerNoteViewsTests(TestCase):
self.assertRedirects(response, reverse("scouting:player_detail", args=[self.player.id])) self.assertRedirects(response, reverse("scouting:player_detail", args=[self.player.id]))
self.assertFalse(PlayerNote.objects.filter(pk=note.id).exists()) self.assertFalse(PlayerNote.objects.filter(pk=note.id).exists())
def test_player_detail_page_shows_notes(self): def test_unauthenticated_user_cannot_add_or_delete_notes(self):
PlayerNote.objects.create(player=self.player, body="Reliable closeout discipline.") add_response = self.client.post(
reverse("scouting:add_note", args=[self.player.id]),
{"body": "Test note"},
)
self.assertRedirects(add_response, f"{reverse('login')}?next={reverse('scouting:add_note', args=[self.player.id])}")
note = PlayerNote.objects.create(user=self.user, player=self.player, body="Existing note")
delete_response = self.client.post(reverse("scouting:delete_note", args=[self.player.id, note.id]))
self.assertRedirects(
delete_response,
f"{reverse('login')}?next={reverse('scouting:delete_note', args=[self.player.id, note.id])}",
)
def test_player_detail_page_shows_only_current_users_notes(self):
PlayerNote.objects.create(user=self.user, player=self.player, body="Reliable closeout discipline.")
PlayerNote.objects.create(user=self.other_user, player=self.player, body="Other scout private note.")
self.client.force_login(self.user)
response = self.client.get(reverse("scouting:player_detail", args=[self.player.id])) response = self.client.get(reverse("scouting:player_detail", args=[self.player.id]))
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
self.assertContains(response, "Scouting Notes") self.assertContains(response, "Scouting Notes")
self.assertContains(response, "Reliable closeout discipline.") self.assertContains(response, "Reliable closeout discipline.")
self.assertNotContains(response, "Other scout private note.")
def test_existing_search_shortlist_and_detail_flows_still_load(self): def test_existing_search_shortlist_and_detail_flows_still_load(self):
FavoritePlayer.objects.create(player=self.player) FavoritePlayer.objects.create(user=self.user, player=self.player)
self.client.force_login(self.user)
search_response = self.client.get(reverse("scouting:player_list")) search_response = self.client.get(reverse("scouting:player_list"))
favorites_response = self.client.get(reverse("scouting:favorites_list")) favorites_response = self.client.get(reverse("scouting:favorites_list"))
@ -533,10 +596,44 @@ class PlayerNoteViewsTests(TestCase):
self.assertEqual(detail_response.status_code, 200) self.assertEqual(detail_response.status_code, 200)
def test_favorites_page_shows_note_count(self): def test_favorites_page_shows_note_count(self):
FavoritePlayer.objects.create(player=self.player) FavoritePlayer.objects.create(user=self.user, player=self.player)
PlayerNote.objects.create(player=self.player, body="Can defend up a position in small lineups.") PlayerNote.objects.create(user=self.user, player=self.player, body="Can defend up a position in small lineups.")
PlayerNote.objects.create(player=self.player, body="Late-clock decision making still inconsistent.") PlayerNote.objects.create(user=self.user, player=self.player, body="Late-clock decision making still inconsistent.")
PlayerNote.objects.create(user=self.other_user, player=self.player, body="Other user's note should not count.")
self.client.force_login(self.user)
response = self.client.get(reverse("scouting:favorites_list")) response = self.client.get(reverse("scouting:favorites_list"))
self.assertContains(response, "Notes: 2") self.assertContains(response, "Notes: 2")
class UserScopedMigrationTests(TransactionTestCase):
reset_sequences = True
migrate_from = [("scouting", "0006_playernote")]
migrate_to = [("scouting", "0007_user_scoped_favorites_and_notes")]
def setUp(self):
super().setUp()
self.executor = MigrationExecutor(connection)
self.executor.migrate(self.migrate_from)
old_apps = self.executor.loader.project_state(self.migrate_from).apps
player_model = old_apps.get_model("scouting", "Player")
favorite_model = old_apps.get_model("scouting", "FavoritePlayer")
note_model = old_apps.get_model("scouting", "PlayerNote")
player = player_model.objects.create(full_name="Legacy Shared Player", position="PG")
favorite_model.objects.create(player=player)
note_model.objects.create(player=player, body="Legacy shared note")
self.executor = MigrationExecutor(connection)
self.executor.migrate(self.migrate_to)
def test_legacy_shared_rows_are_cleared_by_user_scope_migration(self):
apps = self.executor.loader.project_state(self.migrate_to).apps
favorite_model = apps.get_model("scouting", "FavoritePlayer")
note_model = apps.get_model("scouting", "PlayerNote")
self.assertEqual(favorite_model.objects.count(), 0)
self.assertEqual(note_model.objects.count(), 0)

View File

@ -3,7 +3,8 @@ from __future__ import annotations
from decimal import Decimal from decimal import Decimal
from django.core.paginator import Paginator from django.core.paginator import Paginator
from django.db.models import Count, Exists, OuterRef, Prefetch from django.contrib.auth.decorators import login_required
from django.db.models import Count, Exists, OuterRef, Prefetch, Q
from django.http import HttpResponseRedirect from django.http import HttpResponseRedirect
from django.shortcuts import get_object_or_404, render from django.shortcuts import get_object_or_404, render
from django.urls import reverse from django.urls import reverse
@ -28,8 +29,11 @@ CONTEXT_SORTS = {
} }
def apply_favorite_state(players): def apply_favorite_state(players, user):
favorite_ids = set(FavoritePlayer.objects.values_list("player_id", flat=True)) if not user.is_authenticated:
favorite_ids = set()
else:
favorite_ids = set(FavoritePlayer.objects.filter(user=user).values_list("player_id", flat=True))
for player in players: for player in players:
player.is_favorite = player.id in favorite_ids player.is_favorite = player.id in favorite_ids
@ -248,7 +252,7 @@ def player_list(request):
active_sort = sort_players(players, requested_sort, context_filters_used) active_sort = sort_players(players, requested_sort, context_filters_used)
paginator = Paginator(players, PAGE_SIZE) paginator = Paginator(players, PAGE_SIZE)
page_obj = paginator.get_page(request.GET.get("page")) page_obj = paginator.get_page(request.GET.get("page"))
apply_favorite_state(page_obj.object_list) apply_favorite_state(page_obj.object_list, request.user)
query_without_page = request.GET.copy() query_without_page = request.GET.copy()
query_without_page.pop("page", None) query_without_page.pop("page", None)
@ -269,7 +273,7 @@ def player_list(request):
def player_detail(request, player_id: int): def player_detail(request, player_id: int):
player = get_object_or_404( player = get_object_or_404(
Player.objects.prefetch_related("roles", "specialties", "notes"), Player.objects.prefetch_related("roles", "specialties"),
pk=player_id, pk=player_id,
) )
@ -278,7 +282,12 @@ def player_detail(request, player_id: int):
.select_related("season", "team", "competition", "stats") .select_related("season", "team", "competition", "stats")
.order_by("-season__start_year", "team__name", "competition__name") .order_by("-season__start_year", "team__name", "competition__name")
) )
notes = player.notes.all() if request.user.is_authenticated:
notes = player.notes.filter(user=request.user)
is_favorite = FavoritePlayer.objects.filter(user=request.user, player=player).exists()
else:
notes = PlayerNote.objects.none()
is_favorite = False
return render( return render(
request, request,
@ -287,46 +296,52 @@ def player_detail(request, player_id: int):
"player": player, "player": player,
"contexts": contexts, "contexts": contexts,
"notes": notes, "notes": notes,
"is_favorite": FavoritePlayer.objects.filter(player=player).exists(), "is_favorite": is_favorite,
}, },
) )
@login_required
@require_POST @require_POST
def add_favorite(request, player_id: int): def add_favorite(request, player_id: int):
player = get_object_or_404(Player, pk=player_id) player = get_object_or_404(Player, pk=player_id)
FavoritePlayer.objects.get_or_create(player=player) FavoritePlayer.objects.get_or_create(user=request.user, player=player)
return redirect_to_next(request, reverse("scouting:player_detail", args=[player.id])) return redirect_to_next(request, reverse("scouting:player_detail", args=[player.id]))
@login_required
@require_POST @require_POST
def remove_favorite(request, player_id: int): def remove_favorite(request, player_id: int):
player = get_object_or_404(Player, pk=player_id) player = get_object_or_404(Player, pk=player_id)
FavoritePlayer.objects.filter(player=player).delete() FavoritePlayer.objects.filter(user=request.user, player=player).delete()
return redirect_to_next(request, reverse("scouting:player_detail", args=[player.id])) return redirect_to_next(request, reverse("scouting:player_detail", args=[player.id]))
@login_required
@require_POST @require_POST
def add_note(request, player_id: int): def add_note(request, player_id: int):
player = get_object_or_404(Player, pk=player_id) player = get_object_or_404(Player, pk=player_id)
body = (request.POST.get("body") or "").strip() body = (request.POST.get("body") or "").strip()
if body: if body:
PlayerNote.objects.create(player=player, body=body) PlayerNote.objects.create(user=request.user, player=player, body=body)
return redirect_to_next(request, reverse("scouting:player_detail", args=[player.id])) return redirect_to_next(request, reverse("scouting:player_detail", args=[player.id]))
@login_required
@require_POST @require_POST
def delete_note(request, player_id: int, note_id: int): def delete_note(request, player_id: int, note_id: int):
player = get_object_or_404(Player, pk=player_id) player = get_object_or_404(Player, pk=player_id)
PlayerNote.objects.filter(player=player, pk=note_id).delete() PlayerNote.objects.filter(user=request.user, player=player, pk=note_id).delete()
return redirect_to_next(request, reverse("scouting:player_detail", args=[player.id])) return redirect_to_next(request, reverse("scouting:player_detail", args=[player.id]))
@login_required
def favorites_list(request): def favorites_list(request):
favorites = list( favorites = list(
FavoritePlayer.objects.select_related("player") FavoritePlayer.objects.filter(user=request.user)
.select_related("player")
.prefetch_related("player__roles", "player__specialties") .prefetch_related("player__roles", "player__specialties")
.annotate(note_count=Count("player__notes")) .annotate(note_count=Count("player__notes", filter=Q(player__notes__user=request.user)))
.order_by("-created_at", "player__full_name") .order_by("-created_at", "player__full_name")
) )