diff --git a/api/tests.py b/api/tests.py index 5d37276..520943b 100644 --- a/api/tests.py +++ b/api/tests.py @@ -1,9 +1,12 @@ import datetime +import io +from django.core.files.uploadedfile import SimpleUploadedFile from django.test import TestCase, override_settings from django.utils import timezone +from PIL import Image -from club.models import Club, Season, Sponsor +from club.models import Club, ClubMembership, Season, Sponsor from events.models import Event, Location, Opponent from members.models import Member from news.models import News, NewsPhoto @@ -97,6 +100,47 @@ class NewsApiTests(ApiTestBase): self.assertEqual(response.status_code, 200) self.assertEqual(response.json()["results"], []) + def test_excerpt_is_a_truncated_prefix_of_the_body(self): + self.make_news(body=" ".join(f"word{i}" for i in range(80))) + + excerpt = self.api_get("/news/").json()["results"][0]["excerpt"] + + self.assertTrue(excerpt.startswith("word0 word1")) + self.assertTrue(excerpt.endswith("…")) + self.assertLess(len(excerpt.split()), 80) + + def test_excerpt_is_unchanged_when_the_body_is_already_short(self): + item = self.make_news(body="Short body.") + + excerpt = self.api_get("/news/").json()["results"][0]["excerpt"] + + self.assertEqual(excerpt, item.body) + + def test_slug_is_auto_populated_from_the_title(self): + item = self.make_news(title="Big Win This Weekend") + + self.assertEqual(item.slug, "big-win-this-weekend") + + def test_get_single_news_item_by_slug(self): + item = self.make_news(title="Big Win This Weekend") + + response = self.api_get(f"/news/{item.slug}/") + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()["id"], str(item.pk)) + + def test_get_single_news_item_404s_for_an_unknown_slug(self): + response = self.api_get("/news/no-such-item/") + + self.assertEqual(response.status_code, 404) + + def test_get_single_news_item_respects_visibility(self): + item = self.make_news(visibility=News.Visibility.INTERNAL) + + response = self.api_get(f"/news/{item.slug}/") + + self.assertEqual(response.status_code, 404) + class TeamsApiTests(ApiTestBase): def setUp(self): @@ -192,6 +236,23 @@ class TeamsApiTests(ApiTestBase): self.assertEqual(response.status_code, 404) + def test_a_players_license_comes_from_their_club_membership(self): + alice = Member.objects.create(first_name="Alice", last_name="Ash") + TeamMembership.objects.create(team=self.team, member=alice, season=self.season, position=self.forward, jersey_number=2) + ClubMembership.objects.create(club=self.club, member=alice, season=self.season, license="BE-12345") + + player = self.api_get(f"/teams/{self.team.pk}/roster/").json()["players"][0]["players"][0] + + self.assertEqual(player["license"], "BE-12345") + + def test_a_players_license_is_null_without_a_club_membership(self): + alice = Member.objects.create(first_name="Alice", last_name="Ash") + TeamMembership.objects.create(team=self.team, member=alice, season=self.season, position=self.forward, jersey_number=2) + + player = self.api_get(f"/teams/{self.team.pk}/roster/").json()["players"][0]["players"][0] + + self.assertIsNone(player["license"]) + class GamesApiTests(ApiTestBase): def setUp(self): @@ -212,8 +273,8 @@ class GamesApiTests(ApiTestBase): games = self.api_get("/games/upcoming/").json() self.assertEqual(len(games), 1) - self.assertEqual(games[0]["home_team"], "First Team") - self.assertEqual(games[0]["away_team"], "Rivals FC") + self.assertEqual(games[0]["home_team"]["name"], "First Team") + self.assertEqual(games[0]["away_team"]["name"], "Rivals FC") self.assertEqual(games[0]["location"]["name"], "Home Arena") self.assertEqual(games[0]["status"], "upcoming") @@ -289,8 +350,8 @@ class GamesApiTests(ApiTestBase): games = self.api_get(f"/teams/{self.team.pk}/games/").json() - self.assertEqual(games[0]["home_team"], "Rivals FC") - self.assertEqual(games[0]["away_team"], "First Team") + self.assertEqual(games[0]["home_team"]["name"], "Rivals FC") + self.assertEqual(games[0]["away_team"]["name"], "First Team") self.assertEqual(games[0]["home_score"], 3) self.assertEqual(games[0]["away_score"], 4) self.assertEqual(games[0]["status"], "finished") @@ -300,8 +361,8 @@ class GamesApiTests(ApiTestBase): games = self.api_get(f"/teams/{self.team.pk}/games/").json() - self.assertEqual(games[0]["home_team"], "First Team") - self.assertEqual(games[0]["away_team"], "Rivals FC") + self.assertEqual(games[0]["home_team"]["name"], "First Team") + self.assertEqual(games[0]["away_team"]["name"], "Rivals FC") self.assertEqual(games[0]["home_score"], 4) self.assertEqual(games[0]["away_score"], 3) @@ -338,6 +399,36 @@ class GamesApiTests(ApiTestBase): self.assertEqual(response.status_code, 404) + def test_home_team_links_to_the_actual_team_and_the_clubs_logo(self): + # Our own teams have no logo of their own -- they're shown under the club's badge. + self.club.logo = "clubs/ajax-united/logo.png" + self.club.save() + self.make_game(location=self.home_location) + + home_team = self.api_get("/games/upcoming/").json()[0]["home_team"] + + self.assertEqual(home_team["id"], str(self.team.pk)) + self.assertEqual(home_team["name"], "First Team") + self.assertTrue(home_team["logo_url"].startswith("http://ajax-united.rosterchief.app/media/")) + + def test_away_team_links_to_the_opponent_and_its_own_logo(self): + self.opponent.logo = "opponents/rivals.png" + self.opponent.save() + self.make_game(location=self.home_location) + + away_team = self.api_get("/games/upcoming/").json()[0]["away_team"] + + self.assertEqual(away_team["id"], str(self.opponent.pk)) + self.assertEqual(away_team["name"], "Rivals FC") + self.assertTrue(away_team["logo_url"].startswith("http://ajax-united.rosterchief.app/media/")) + + def test_team_logo_url_is_null_without_a_club_logo(self): + self.make_game(location=self.home_location) + + home_team = self.api_get("/games/upcoming/").json()[0]["home_team"] + + self.assertIsNone(home_team["logo_url"]) + class TenancyAndCorsTests(ApiTestBase): def test_the_base_domain_404s(self): @@ -432,6 +523,44 @@ class SponsorApiTests(ApiTestBase): self.assertIsNone(self.api_get("/sponsors/").json()[0]["logo_url"]) + def test_logo_dimensions_are_computed_for_a_raster_image(self): + buffer = io.BytesIO() + Image.new("RGB", (300, 150)).save(buffer, format="PNG") + logo = SimpleUploadedFile("logo.png", buffer.getvalue(), content_type="image/png") + + self.make_sponsor(logo=logo) + + sponsor = self.api_get("/sponsors/").json()[0] + self.assertEqual(sponsor["logo_width"], 300) + self.assertEqual(sponsor["logo_height"], 150) + + def test_logo_dimensions_are_computed_for_an_svg_with_width_and_height(self): + svg = b'' + logo = SimpleUploadedFile("logo.svg", svg, content_type="image/svg+xml") + + self.make_sponsor(logo=logo) + + sponsor = self.api_get("/sponsors/").json()[0] + self.assertEqual(sponsor["logo_width"], 120) + self.assertEqual(sponsor["logo_height"], 80) + + def test_logo_dimensions_fall_back_to_an_svg_viewbox(self): + svg = b'' + logo = SimpleUploadedFile("logo.svg", svg, content_type="image/svg+xml") + + self.make_sponsor(logo=logo) + + sponsor = self.api_get("/sponsors/").json()[0] + self.assertEqual(sponsor["logo_width"], 64) + self.assertEqual(sponsor["logo_height"], 32) + + def test_logo_dimensions_are_null_without_a_logo(self): + self.make_sponsor() + + sponsor = self.api_get("/sponsors/").json()[0] + self.assertIsNone(sponsor["logo_width"]) + self.assertIsNone(sponsor["logo_height"]) + def test_randomize_returns_the_same_set_of_sponsors(self): for i in range(5): self.make_sponsor(name=f"Sponsor {i}") diff --git a/club/api.py b/club/api.py index 71e2e28..84b7179 100644 --- a/club/api.py +++ b/club/api.py @@ -21,6 +21,8 @@ class SponsorOut(Schema): id: uuid.UUID name: str logo_url: str | None + logo_width: int | None + logo_height: int | None url: str | None start_date: date end_date: date | None @@ -31,6 +33,8 @@ def _to_sponsor_out(sponsor, request) -> SponsorOut: id=sponsor.pk, name=sponsor.name, logo_url=request.build_absolute_uri(sponsor.logo.url) if sponsor.logo else None, + logo_width=sponsor.logo_width, + logo_height=sponsor.logo_height, url=sponsor.url or None, start_date=sponsor.start_date, end_date=sponsor.end_date, diff --git a/club/migrations/0020_sponsor_logo_height_sponsor_logo_width.py b/club/migrations/0020_sponsor_logo_height_sponsor_logo_width.py new file mode 100644 index 0000000..760fb6a --- /dev/null +++ b/club/migrations/0020_sponsor_logo_height_sponsor_logo_width.py @@ -0,0 +1,35 @@ +# Generated by Django 6.0.6 on 2026-08-07 14:26 + +from django.db import migrations, models + +from club.services.images import get_image_dimensions + + +def backfill_logo_dimensions(apps, schema_editor): + """Existing sponsors uploaded a logo before these fields existed, so + Sponsor.save()'s new dimension computation never ran for them.""" + Sponsor = apps.get_model("club", "Sponsor") + for sponsor in Sponsor.objects.exclude(logo=""): + width, height = get_image_dimensions(sponsor.logo) + Sponsor.objects.filter(pk=sponsor.pk).update(logo_width=width, logo_height=height) + + +class Migration(migrations.Migration): + + dependencies = [ + ('club', '0019_sponsor'), + ] + + operations = [ + migrations.AddField( + model_name='sponsor', + name='logo_height', + field=models.PositiveIntegerField(blank=True, editable=False, null=True, verbose_name='logo height'), + ), + migrations.AddField( + model_name='sponsor', + name='logo_width', + field=models.PositiveIntegerField(blank=True, editable=False, null=True, verbose_name='logo width'), + ), + migrations.RunPython(backfill_logo_dimensions, migrations.RunPython.noop), + ] diff --git a/club/models.py b/club/models.py index 4123466..982ba7b 100644 --- a/club/models.py +++ b/club/models.py @@ -174,6 +174,12 @@ class Sponsor(ClubScopedModel): # Pillow validation can't read those. validators=[FileExtensionValidator(allowed_extensions=["png", "jpg", "jpeg", "gif", "webp", "svg"])], ) + # Not user-editable: recomputed from the logo file itself on every save, same reasoning + # NewsPhoto/TeamPhoto don't need this -- FileField (not ImageField) means Django never + # populates width/height on its own. The public API exposes these so a consumer can lay + # out a sponsor strip without waiting on the image to load. + logo_width = models.PositiveIntegerField(_("logo width"), null=True, blank=True, editable=False) + logo_height = models.PositiveIntegerField(_("logo height"), null=True, blank=True, editable=False) url = models.URLField(_("URL"), blank=True, help_text=_("The sponsor's own website, if they have one.")) start_date = models.DateField(_("start date")) @@ -191,6 +197,14 @@ class Sponsor(ClubScopedModel): if self.end_date is not None and self.start_date is not None and self.end_date < self.start_date: raise ValidationError({"end_date": _("End date can't be before the start date.")}) + def save(self, *args, **kwargs): + # Deferred: club.services (via its __init__) imports back from club.models, so a + # module-level import here would be circular. + from club.services.images import get_image_dimensions + + self.logo_width, self.logo_height = get_image_dimensions(self.logo) if self.logo else (None, None) + super().save(*args, **kwargs) + class Season(ClubScopedModel): start_date = models.DateField(_("start date")) diff --git a/club/services/images.py b/club/services/images.py new file mode 100644 index 0000000..d279c36 --- /dev/null +++ b/club/services/images.py @@ -0,0 +1,82 @@ +"""Dimensions for uploads that aren't Django ImageFields. + +Logos (Club.logo, Sponsor.logo) are plain FileFields, not ImageFields -- +Pillow can't validate SVGs, and crests/sponsor logos are commonly vector +files -- so there's no automatic width_field/height_field the way there +would be on an ImageField. This fills that gap: Pillow for raster formats, +a bounded regex read of the root tag for vector ones (not a full XML +parse -- this reads untrusted uploads, and a parser is exposed to entity +expansion attacks a plain attribute read never is). +""" + +import re + +from PIL import Image, UnidentifiedImageError + +_SVG_TAG_RE = re.compile(rb"]*>", re.IGNORECASE | re.DOTALL) +_WIDTH_RE = re.compile(rb"""\bwidth\s*=\s*["']([^"']+)["']""", re.IGNORECASE) +_HEIGHT_RE = re.compile(rb"""\bheight\s*=\s*["']([^"']+)["']""", re.IGNORECASE) +_VIEWBOX_RE = re.compile(rb"""\bviewBox\s*=\s*["']\s*([\d.+-]+)[ ,]+([\d.+-]+)[ ,]+([\d.+-]+)[ ,]+([\d.+-]+)""", re.IGNORECASE) +_LEADING_NUMBER_RE = re.compile(r"[\d.]+") + +#: The root tag is always near the top of the file -- no need to read +#: (or regex-scan) anything past a small header. +_SVG_HEAD_BYTES = 8192 + + +def _svg_length(raw: bytes) -> int | None: + """Parse an SVG length attribute (``"200"``, ``"200px"``) to a rounded + int, or None if it's relative (``"100%"``) and so not a real pixel size.""" + text = raw.decode("utf-8", errors="ignore").strip() + if text.endswith("%"): + return None + match = _LEADING_NUMBER_RE.match(text) + return round(float(match.group(0))) if match else None + + +def _svg_dimensions(file) -> tuple[int | None, int | None]: + try: + file.seek(0) + head = file.read(_SVG_HEAD_BYTES) + except OSError: + return None, None + # Reset for whatever reads the file next (e.g. FileField writing it to storage). + file.seek(0) + + tag_match = _SVG_TAG_RE.search(head) + svg_tag = tag_match.group(0) if tag_match else head + + width_match, height_match = _WIDTH_RE.search(svg_tag), _HEIGHT_RE.search(svg_tag) + if width_match and height_match: + width, height = _svg_length(width_match.group(1)), _svg_length(height_match.group(1)) + if width and height: + return width, height + + viewbox_match = _VIEWBOX_RE.search(svg_tag) + if viewbox_match: + _, _, width, height = viewbox_match.groups() + return round(float(width)), round(float(height)) + + return None, None + + +def get_image_dimensions(file) -> tuple[int | None, int | None]: + """Best-effort (width, height) for an uploaded logo -- (None, None) if the + file can't be read as an image (corrupt upload, unrecognised format).""" + if not file: + return None, None + + name = getattr(file, "name", "") or "" + if name.lower().endswith(".svg"): + return _svg_dimensions(file) + + try: + file.seek(0) + with Image.open(file) as image: + size = image.size + # Reset for whatever reads the file next (e.g. FileField writing it to storage) -- + # only on the success path, since a failed open/read leaves nothing to rewind. + file.seek(0) + return size + except (OSError, UnidentifiedImageError): + return None, None diff --git a/events/api.py b/events/api.py index 5df94f9..f68987a 100644 --- a/events/api.py +++ b/events/api.py @@ -40,12 +40,18 @@ class LocationOut(Schema): is_home: bool +class TeamRefOut(Schema): + id: uuid.UUID + name: str + logo_url: str | None + + class GameOut(Schema): id: uuid.UUID start: datetime location: LocationOut | None - home_team: str | None - away_team: str | None + home_team: TeamRefOut | None + away_team: TeamRefOut | None competition: str is_live: bool status: str # "upcoming" | "live" | "finished" @@ -53,21 +59,27 @@ class GameOut(Schema): away_score: int | None -def _to_game_out(event, team=None) -> GameOut: +def _to_team_ref_out(request, *, id, name, logo) -> TeamRefOut: + return TeamRefOut(id=id, name=name, logo_url=request.build_absolute_uri(logo.url) if logo else None) + + +def _to_game_out(event, request, club, team=None) -> GameOut: if team is None: # .first() would re-query even with teams prefetched; go through the # prefetch cache instead. related_teams = list(event.teams.all()) team = related_teams[0] if related_teams else None - team_name = team.name if team is not None else None - opponent_name = event.opponent.name if event.opponent_id else None + # Our own team has no logo of its own -- it's shown under the club's badge. The + # opponent is an events.models.Opponent, which does carry its own logo. + team_ref = _to_team_ref_out(request, id=team.pk, name=team.name, logo=club.logo) if team is not None else None + opponent_ref = _to_team_ref_out(request, id=event.opponent_id, name=event.opponent.name, logo=event.opponent.logo) if event.opponent_id else None if event.is_home_game: - home_team, away_team = team_name, opponent_name + home_team, away_team = team_ref, opponent_ref home_score, away_score = event.score_for, event.score_against else: - home_team, away_team = opponent_name, team_name + home_team, away_team = opponent_ref, team_ref home_score, away_score = event.score_against, event.score_for if event.is_live: @@ -103,7 +115,7 @@ def list_upcoming_games(request, count: int = DEFAULT_UPCOMING_COUNT): events = Event.objects.filter(club=club, kind__in=UPCOMING_KINDS, cancelled=False, start__gte=timezone.now()).select_related("opponent", "location").prefetch_related("teams").order_by("start")[:count] - return [_to_game_out(event) for event in events] + return [_to_game_out(event, request, club) for event in events] @router.get("/games/live/", response=list[GameOut], summary="Live games") @@ -112,7 +124,7 @@ def list_live_games(request): events = Event.objects.filter(club=club, kind=Event.EventKind.GAME, cancelled=False, is_live=True).select_related("opponent", "location").prefetch_related("teams").order_by("start") - return [_to_game_out(event) for event in events] + return [_to_game_out(event, request, club) for event in events] @router.get("/teams/{team_id}/games/", response=list[GameOut], summary="A team's current-season games") @@ -136,4 +148,4 @@ def list_team_games(request, team_id: uuid.UUID): .order_by("start") ) - return [_to_game_out(event, team=team) for event in events] + return [_to_game_out(event, request, club, team=team) for event in events] diff --git a/news/api.py b/news/api.py index b4e84b5..ed59e9d 100644 --- a/news/api.py +++ b/news/api.py @@ -11,7 +11,9 @@ import uuid from datetime import datetime from django.utils import timezone +from django.utils.text import Truncator from ninja import Router, Schema +from ninja.errors import HttpError from api.errors import require_club @@ -22,6 +24,11 @@ router = Router(tags=["news"]) DEFAULT_LIMIT = 20 MAX_LIMIT = 100 +#: Words, not characters -- reads more naturally cut off mid-list than a hard +#: character count, and body is plain text so there's no markup to worry about +#: truncating mid-tag. +EXCERPT_WORDS = 40 + class NewsPhotoOut(Schema): url: str @@ -33,6 +40,7 @@ class NewsItemOut(Schema): id: uuid.UUID title: str slug: str + excerpt: str body: str published_at: datetime teams: list[str] @@ -46,11 +54,21 @@ class NewsListOut(Schema): results: list[NewsItemOut] +def _visible_news(club): + return News.objects.filter( + club=club, + status=News.Status.PUBLISHED, + published_at__lte=timezone.now(), + visibility__in=[News.Visibility.EXTERNAL, News.Visibility.BOTH], + ) + + def _to_news_item_out(item, request) -> NewsItemOut: return NewsItemOut( id=item.pk, title=item.title, slug=item.slug, + excerpt=Truncator(item.body).words(EXCERPT_WORDS, truncate=" …"), body=item.body, published_at=item.published_at, teams=[team.name for team in item.teams.all()], @@ -66,18 +84,19 @@ def list_news(request, limit: int = DEFAULT_LIMIT, offset: int = 0): limit = max(1, min(limit, MAX_LIMIT)) offset = max(0, offset) - queryset = ( - News.objects.filter( - club=club, - status=News.Status.PUBLISHED, - published_at__lte=timezone.now(), - visibility__in=[News.Visibility.EXTERNAL, News.Visibility.BOTH], - ) - .prefetch_related("photos", "teams") - .order_by("-published_at") - ) + queryset = _visible_news(club).prefetch_related("photos", "teams").order_by("-published_at") count = queryset.count() page = queryset[offset : offset + limit] return NewsListOut(count=count, limit=limit, offset=offset, results=[_to_news_item_out(item, request) for item in page]) + + +@router.get("/{slug}/", response=NewsItemOut, summary="Single published news item") +def get_news_item(request, slug: str): + club = require_club(request) + item = _visible_news(club).filter(slug=slug).prefetch_related("photos", "teams").first() + if item is None: + raise HttpError(404, "No such news item.") + + return _to_news_item_out(item, request) diff --git a/news/migrations/0002_backfill_slugs.py b/news/migrations/0002_backfill_slugs.py new file mode 100644 index 0000000..a09a962 --- /dev/null +++ b/news/migrations/0002_backfill_slugs.py @@ -0,0 +1,23 @@ +from django.db import migrations + +from rosterchief.base import unique_slugify + + +def backfill_slugs(apps, schema_editor): + """News.save() already auto-populates a blank slug (rosterchief.base. + ClubScopedModel.save), but that only fires on save() -- rows created + before the slug field existed, or via bulk_create, never got one.""" + News = apps.get_model("news", "News") + for item in News.objects.filter(slug=""): + item.slug = unique_slugify(item, item.title, scope={"club_id": item.club_id}) + item.save(update_fields=["slug"]) + + +class Migration(migrations.Migration): + dependencies = [ + ("news", "0001_initial"), + ] + + operations = [ + migrations.RunPython(backfill_slugs, migrations.RunPython.noop), + ] diff --git a/teams/api.py b/teams/api.py index 6853ea7..8ef54c1 100644 --- a/teams/api.py +++ b/teams/api.py @@ -11,6 +11,7 @@ from ninja import Router, Schema from ninja.errors import HttpError from api.errors import require_club +from club.models import ClubMembership from club.services.access import current_season from .models import Team, TeamMembership, TeamPhoto @@ -32,6 +33,7 @@ class PlayerOut(Schema): jersey_number: int | None is_captain: bool is_alternate_captain: bool + license: str | None class PositionGroupOut(Schema): @@ -58,7 +60,7 @@ def _to_team_out(team, request, photo=None) -> TeamOut: return TeamOut(id=team.pk, name=team.name, short_name=team.short_name, photo_url=photo_url) -def _to_player_out(membership) -> PlayerOut: +def _to_player_out(membership, license_by_member_id) -> PlayerOut: return PlayerOut( id=membership.member_id, first_name=membership.member.first_name, @@ -66,6 +68,7 @@ def _to_player_out(membership) -> PlayerOut: jersey_number=membership.jersey_number, is_captain=membership.is_captain, is_alternate_captain=membership.is_alternate_captain, + license=license_by_member_id.get(membership.member_id) or None, ) @@ -84,9 +87,13 @@ def build_roster(team, request) -> RosterOut: memberships = TeamMembership.objects.filter(team=team, season=season).select_related("member", "position").order_by("position__ordering", "position__name", "jersey_number") assignments = team.staff_assignments.filter(season=season).select_related("member", "position").order_by("position__ordering", "position__name", "member__last_name") + # A player's license lives on their club-wide ClubMembership for the season, not on + # TeamMembership -- one query for all of them rather than one per player. + license_by_member_id = dict(ClubMembership.objects.filter(club=team.club, season=season).values_list("member_id", "license")) + # groupby only groups consecutive runs -- relies on the queryset already # being ordered by position first, which it is. - players = [PositionGroupOut(position=position_name, players=[_to_player_out(m) for m in members]) for position_name, members in groupby(memberships, key=lambda m: m.position.name)] + players = [PositionGroupOut(position=position_name, players=[_to_player_out(m, license_by_member_id) for m in members]) for position_name, members in groupby(memberships, key=lambda m: m.position.name)] staff = [ StaffMemberOut(id=assignment.member_id, first_name=assignment.member.first_name, last_name=assignment.member.last_name, position=assignment.position.name) for assignment in assignments