Add spacing, a larger body, and photo uploads to the coach news form

The post-composer screen was cramped and text-only, and its "audience"
picker exposed the same "which team(s)" choice the create-event screen
already dropped -- hard-lock teams to the coach's active team instead
(hidden field, required=True so a tampered empty submission can't
silently become club-wide), matching how CoachCreateEventView.teams
already works. Photos reuse management's own NewsPhotoUploadForm/
NewsPhoto machinery, first upload becomes the main picture.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

mobile/coach_views.py

@@ -29,16 +29,16 @@ from events.services.attendance import member_attendance_counts, record_check_in
 from events.services.calendar import agenda_groups
 from events.services.lineup import UNAVAILABLE_STATUSES, cancel_scheduled_publish, publish_lineup, schedule_lineup_publish, toggle_selection
 from events.tasks import notify_new_event
-from management.forms import EventForm, EventSeriesForm, LocationForm, NewsForm, OpponentForm
+from management.forms import EventForm, EventSeriesForm, LocationForm, NewsForm, NewsPhotoUploadForm, OpponentForm
 from members.models import Member
-from news.models import News
+from news.models import News, NewsPhoto
 from news.services import notify_editors_of_pending_review
 from notifications.services import notify_members
 from teams.models import Position, StaffAssignment, Team, TeamMembership
 from teams.services import eligible_roster_members
 
 from .coach_mixins import CoachScopeMixin
-from .forms import _INPUT_CLASSES, CoachRosterEditForm
+from .forms import _INPUT_CLASSES, _TEXTAREA_CLASSES, CoachRosterEditForm
 
 #: RSVP states that count as "in" for the stat tile -- present/selected are an
 #: explicit yes, maybe is still a lean-in rather than silence.
@@ -404,10 +404,15 @@ class CoachCreateEventView(CoachScopeMixin, LoginRequiredMixin, TemplateView):
         #
         # teams: hard-locked to the active team, not a picker -- see this
         # view's own docstring. A hidden MultipleHiddenInput renders its
-        # `initial` on GET without any template code, and the queryset
+        # `initial` on GET without any template code; the queryset
-        # restriction means a tampered request still can't set another team.
+        # restriction means a tampered request can't set a *different* team,
+        # and required=True (Event.teams itself is blank=True, so the form
+        # field defaults to optional) means a tampered request can't submit
+        # an empty selection either -- both would otherwise still produce a
+        # real, saveable event, just not one scoped to a single team anymore.
         form.fields["teams"].widget = forms.MultipleHiddenInput()
         form.fields["teams"].queryset = Team.objects.filter(pk=self.active_team.pk) if self.active_team is not None else Team.objects.none()
+        form.fields["teams"].required = True
         # form.initial (not just field.initial) -- ModelForm.__init__ already
         # populated form.initial["teams"] = [] from the new, unsaved Event/
         # EventSeries instance's own (necessarily empty) m2m, and
@@ -587,10 +592,12 @@ class CoachOpponentCreateView(CoachScopeMixin, LoginRequiredMixin, View):
 
 class CoachCreateNewsView(CoachScopeMixin, LoginRequiredMixin, TemplateView):
     """C5 -- reuses management.forms.NewsForm, re-scoped to the coach's own
-    managed team(s). NewsForm.__init__ defaults ``teams`` to every club team
+    active team. ``teams`` isn't a picker at all here (same reasoning as
-    -- fine for an editor/admin, but a real gap for a coach, who should only
+    CoachCreateEventView's own ``teams`` -- there's no "which team" question
-    ever be able to post as their own team, never "on behalf of" one they
+    on a screen already scoped to one): hard-locked to the active team, a
-    don't run.
+    hidden field an editor can still widen from the desktop when reviewing
+    the submission, which is the honest place for that judgment call to live
+    -- not a checkbox list a coach has to get right on a phone.
 
     Gated with club.services.access.can_add_news, which already includes
     is_coach_manager -- no new authorization logic needed. On submit, the
@@ -599,19 +606,20 @@ class CoachCreateNewsView(CoachScopeMixin, LoginRequiredMixin, TemplateView):
     NewsSubmitForReviewView makes) rather than left a silent draft, so it
     actually reaches an editor's queue -- can_publish_news stays editor/
     admin-only, so "Send for review" is the honest label here, not "Publish"
-    the way the design mock has it.
+    the way the design mock has it. Any uploaded photos are attached via
+    management.forms.NewsPhotoUploadForm's own "images" field and
+    NewsPhotoUploadView's own first-upload-becomes-main logic, reused
+    directly rather than a second copy of either.
 
-    title_en/body_en (the optional English fallback) and a cover photo
+    title_en/body_en (the optional English fallback) aren't part of this
-    aren't part of this screen -- both text fields are blank=True on the
+    screen -- both are blank=True on the model (a coach posting from their
-    model (a coach posting from their phone isn't expected to also draft an
+    phone isn't expected to also draft an English translation).
-    English translation), and News has no image field for a cover at all to
+    ``visibility`` is left at the model's own default (INTERNAL -- team
-    begin with. ``visibility`` is left at the model's own default
+    families, in-app only) rather than building the mock's "also on club
-    (INTERNAL -- team families, in-app only) rather than building the mock's
+    website" toggle: a coach's post always lands as PENDING_REVIEW first,
-    "also on club website" toggle: a coach's post always lands as
+    and an editor reviewing it can widen visibility before publishing if a
-    PENDING_REVIEW first, and an editor reviewing it can widen visibility
+    public-site placement is actually warranted -- that's a real gate, not a
-    before publishing if a public-site placement is actually warranted --
+    decorative row, so it isn't reproduced here as one.
-    that's a real gate, not a decorative row, so it isn't reproduced here as
-    one.
     """
 
     template_name = "mobile/coach/news_form.html"
@@ -629,35 +637,42 @@ class CoachCreateNewsView(CoachScopeMixin, LoginRequiredMixin, TemplateView):
         del form.fields["title_en"]
         del form.fields["body_en"]
         del form.fields["visibility"]
-        # A coach's post is always about their own team(s) -- never empty
+        # teams: hard-locked to the active team, not a picker -- see this
-        # (which News.teams's own help_text defines as "club-wide", an
+        # view's own docstring. Widget swapped in *before* .queryset is set --
-        # editor/admin-only claim), and never any other team in the club.
+        # ModelChoiceField.queryset's setter pushes `self.widget.choices =
-        #
+        # self.choices` as a side effect at the moment it's assigned, and
-        # Widget swapped in *before* .queryset is set -- ModelChoiceField.
+        # setting the queryset first (then swapping the widget after) would
-        # queryset's setter pushes `self.widget.choices = self.choices` as a
+        # discard that push -- harmless for MultipleHiddenInput specifically
-        # side effect at the moment it's assigned; setting the queryset first
+        # (it renders from `value`/`initial`, not `choices`), but kept in
-        # and swapping the widget after discards that push, leaving the new
+        # this order anyway for consistency with the ModelMultipleChoiceField
-        # widget's own .choices empty and the checkbox list rendering nothing
+        # fields that *do* need it (CoachCreateEventView._scope_shared_fields'
-        # at all (a real, previously-undetected bug -- see the regression
+        # own comment has the full mechanism).
-        # test, and mobile.coach_views.CoachCreateEventView._scope_shared_
+        form.fields["teams"].widget = forms.MultipleHiddenInput()
-        # fields' own near-identical comment for the ModelMultipleChoiceField
+        form.fields["teams"].queryset = Team.objects.filter(pk=self.active_team.pk) if self.active_team is not None else Team.objects.none()
-        # version of the same trap).
+        # News.teams is blank=True, so the auto-generated field defaults to
-        form.fields["teams"].widget = forms.CheckboxSelectMultiple()
+        # required=False -- without this, a tampered empty submission would
-        form.fields["teams"].queryset = Team.objects.filter(pk__in=[team.pk for team in self.managed_teams])
+        # still save, silently becoming a club-wide post (see the analogous
+        # fix and full comment on CoachCreateEventView._scope_shared_fields'
+        # own "teams" field).
         form.fields["teams"].required = True
-        if self.active_team is not None and data is None:
+        # form.initial, not field.initial -- ModelForm.__init__ already set
-            # form.initial, not field.initial -- ModelForm.__init__ already set
+        # form.initial["teams"] = [] from the new, unsaved News instance's
-            # form.initial["teams"] = [] from the new, unsaved News instance's
+        # own (necessarily empty) m2m, and dict.get(name, field.initial)
-            # own (necessarily empty) m2m, and dict.get(name, field.initial)
+        # only falls back to field.initial when the key is *absent*, not
-            # only falls back to field.initial when the key is *absent*, not
+        # merely empty, so field.initial alone silently renders unchecked.
-            # merely empty, so field.initial alone silently renders unchecked.
+        form.initial["teams"] = [self.active_team.pk] if self.active_team is not None else []
-            form.initial["teams"] = [self.active_team.pk]
+        form.fields["title"].widget.attrs["class"] = _INPUT_CLASSES
-        for field_name in ("title", "body"):
+        form.fields["body"].widget.attrs["class"] = _TEXTAREA_CLASSES
-            form.fields[field_name].widget.attrs["class"] = _INPUT_CLASSES
         return form
 
+    def build_photo_form(self, data=None, files=None):
+        photo_form = NewsPhotoUploadForm(data, files)
+        photo_form.fields["images"].required = False
+        return photo_form
+
     def get_context_data(self, **kwargs):
         kwargs.setdefault("form", self.build_form())
+        kwargs.setdefault("photo_form", self.build_photo_form())
         return super().get_context_data(**kwargs)
 
     def post(self, request, *args, **kwargs):
@@ -665,10 +680,13 @@ class CoachCreateNewsView(CoachScopeMixin, LoginRequiredMixin, TemplateView):
             return HttpResponseForbidden()
 
         form = self.build_form(request.POST)
-        if not form.is_valid():
+        photo_form = self.build_photo_form(request.POST, request.FILES)
-            return self.render_to_response(self.get_context_data(form=form))
+        if not form.is_valid() or not photo_form.is_valid():
+            return self.render_to_response(self.get_context_data(form=form, photo_form=photo_form))
 
         news_item = form.save()
+        for index, image in enumerate(photo_form.cleaned_data["images"]):
+            NewsPhoto.objects.create(news_item=news_item, image=image, is_main=index == 0)
         news_item.submit_for_review()
         notify_editors_of_pending_review(news_item)
 

mobile/forms.py

@@ -10,6 +10,12 @@ from teams.models import Position, TeamMembership
 #: form so far just bakes its own classes straight into the widget.
 _INPUT_CLASSES = "h-11 w-full rounded-lg border border-stroke bg-paper px-3 text-[15px] text-ink placeholder:text-dim focus:border-ink focus:outline-none"
 
+#: Same look as _INPUT_CLASSES, sized for a multi-line body instead of a
+#: single-line value -- a fixed height (not h-11) plus vertical padding a
+#: fixed-height input doesn't need, and resize-y so a longer post isn't
+#: stuck scrolling inside a short box.
+_TEXTAREA_CLASSES = "h-40 w-full resize-y rounded-lg border border-stroke bg-paper px-3 py-2.5 text-[15px] text-ink placeholder:text-dim focus:border-ink focus:outline-none"
+
 
 class MemberProfileForm(forms.ModelForm):
     """M6 -- "Edit personal info" (design_handoff_rosterchief_platform/README.md).

mobile/templates/mobile/coach/news_form.html

@@ -3,10 +3,10 @@
 
 {% comment %}
     C5 -- design_handoff_rosterchief_platform/README.md's C5 section: title,
-    body, and which of the coach's own teams this is for. See
+    body, photos, posted for the coach's active team. See CoachCreateNewsView's
-    CoachCreateNewsView's own docstring for what's scoped down from the mock
+    own docstring for what's scoped down from the mock (no English fallback,
-    (no cover photo -- News has no such field; no English fallback; no
+    no audience/visibility toggle -- teams is a hidden field locked to the
-    audience/visibility toggle) and why the button reads "Send for review"
+    active team, not a picker) and why the button reads "Send for review"
     rather than the mock's "Publish" -- a coach's post always needs an
     editor's sign-off first.
 {% endcomment %}
@@ -18,14 +18,16 @@
         <button class="font-display text-sm font-extrabold tracking-wide text-club uppercase" type="submit" form="coach-news-form">{% trans "Send" %}</button>
     </div>
 
-    {% if form.non_field_errors %}
+    {% for error in form.non_field_errors %}
-        <div class="m-card border border-danger-border bg-danger-bg p-3 text-sm text-club-dark">
+        <div class="m-card border border-danger-border bg-danger-bg p-3 text-sm text-club-dark"><p>{{ error }}</p></div>
-            {% for error in form.non_field_errors %}<p>{{ error }}</p>{% endfor %}
+    {% endfor %}
-        </div>
+    {% for error in photo_form.non_field_errors %}
-    {% endif %}
+        <div class="m-card border border-danger-border bg-danger-bg p-3 text-sm text-club-dark"><p>{{ error }}</p></div>
+    {% endfor %}
 
-    <form id="coach-news-form" method="post" action="{% url "mobile:coach_create_news" %}" hx-boost="false">
+    <form id="coach-news-form" class="flex flex-col gap-4" method="post" action="{% url "mobile:coach_create_news" %}" enctype="multipart/form-data" hx-boost="false">
         {% csrf_token %}
+        {{ form.teams }}
 
         <div class="m-card flex flex-col p-4">
             <div>
@@ -42,17 +44,12 @@
         </div>
 
         <div class="m-card p-4">
-            <div class="mb-2 font-display text-xs font-extrabold tracking-wide text-muted uppercase">{% trans "Audience" %}</div>
+            <label class="mb-1 block text-xs font-semibold text-muted" for="{{ photo_form.images.id_for_label }}">{% trans "Photos" %}</label>
-            <div class="flex flex-col gap-2">
+            {{ photo_form.images }}
-                {% for checkbox in form.teams %}
+            <p class="mt-1 text-xs text-dim">{% trans "Optional. The first photo becomes the main picture." %}</p>
-                    <label class="flex items-center gap-2 text-sm text-ink">
+            {% for error in photo_form.images.errors %}<p class="mt-1 text-xs text-club-dark">{{ error }}</p>{% endfor %}
-                        {{ checkbox.tag }}
-                        {{ checkbox.choice_label }}
-                    </label>
-                {% endfor %}
-            </div>
-            {% for error in form.teams.errors %}<p class="mt-1 text-xs text-club-dark">{{ error }}</p>{% endfor %}
-            <p class="mt-2 text-xs text-dim">{% trans "Sent to an editor for review before it goes out to families." %}</p>
         </div>
+
+        <p class="px-1 text-xs text-dim">{% blocktrans with team=active_team.name %}Posted for {{ team }} and sent to an editor for review before it goes out to families -- they can widen who it reaches if needed.{% endblocktrans %}</p>
     </form>
 {% endblock content %}

mobile/tests.py

@@ -3963,13 +3963,15 @@ class CoachCreateNewsViewTests(TestCase):
         self.assertEqual(team_choices, [self.team])
         self.assertNotIn(other_team, team_choices)
 
-    def test_teams_field_is_prechecked_with_the_active_team(self):
+    def test_teams_is_hidden_and_locked_to_the_active_team(self):
         self.client.force_login(self.user)
 
         response = self.client.get(reverse("mobile:coach_create_news"), HTTP_HOST="ajax-united.rosterchief.app")
 
+        self.assertIsInstance(response.context["form"].fields["teams"].widget, forms.MultipleHiddenInput)
         self.assertEqual(response.context["form"].initial.get("teams"), [self.team.pk])
-        self.assertContains(response, f'value="{self.team.pk}" id="id_teams_0" checked')
+        self.assertContains(response, f'<input type="hidden" name="teams" value="{self.team.pk}"')
+        self.assertNotContains(response, 'name="teams" type="checkbox"')
 
     def test_valid_post_creates_a_pending_review_post_scoped_to_the_team(self):
         self.client.force_login(self.user)
@@ -3993,6 +3995,31 @@ class CoachCreateNewsViewTests(TestCase):
         self.assertFalse(News.objects.filter(title="No team picked").exists())
         self.assertTrue(response.context["form"].errors)
 
+    def test_photos_are_optional(self):
+        self.client.force_login(self.user)
+
+        response = self._post(title="No photos here")
+
+        self.assertRedirects(response, reverse("mobile:coach_today"), fetch_redirect_response=False)
+        news_item = News.objects.get(title="No photos here")
+        self.assertEqual(news_item.photos.count(), 0)
+
+    def test_uploaded_photos_are_attached_first_one_is_main(self):
+        self.client.force_login(self.user)
+
+        response = self.client.post(
+            reverse("mobile:coach_create_news"),
+            {"title": "Photo finish", "body": "Great game everyone.", "teams": [str(self.team.pk)], "images": [make_image_file("one.png"), make_image_file("two.png")]},
+            HTTP_HOST="ajax-united.rosterchief.app",
+        )
+
+        self.assertRedirects(response, reverse("mobile:coach_today"), fetch_redirect_response=False)
+        news_item = News.objects.get(title="Photo finish")
+        photos = list(news_item.photos.order_by("ordering"))
+        self.assertEqual(len(photos), 2)
+        self.assertTrue(photos[0].is_main)
+        self.assertFalse(photos[1].is_main)
+
     def test_non_managing_staff_cannot_post(self):
         physio_position = Position.objects.create(club=self.club, name="Physio", short_name="PHY", staff_position=True, management_position=False)
         physio_user = User.objects.create_user(email="physio@example.com", password="pw-secret-123")

static/css/app.css

@@ -3948,6 +3948,9 @@
   .h-24 {
     height: calc(var(--spacing) * 24);
   }
+  .h-40 {
+    height: calc(var(--spacing) * 40);
+  }
   .h-44 {
     height: calc(var(--spacing) * 44);
   }
@@ -4311,6 +4314,9 @@
   .resize {
     resize: both;
   }
+  .resize-y {
+    resize: vertical;
+  }
   .scrollbar-gutter-stable {
     scrollbar-gutter: stable;
   }