From 366239e60bb52a0864c9655c6e9f3de93f9425b5 Mon Sep 17 00:00:00 2001 From: Bernard Siebens Date: Sat, 22 Aug 2026 14:59:23 +0200 Subject: [PATCH] Fix RSVP buttons broken by hx-boost; require a real Out reason; add optional Maybe reason The In/Out hero buttons (and Out's reason confirm) stopped working: Alpine owns the toggle between a row's two sibling forms (buttons vs. the reason prompt), and hx-boost="true" on had htmx *also* intercepting the same submit -- both ended up fighting over it. Fixed by marking every write-action
across the mobile app hx-boost="false" (link navigation, where the smooth-navigation feature actually matters, is untouched). The one exception worth calling out: coach/lineup.html's form uses three submit buttons sharing one via formaction overrides -- htmx's boost reads the form's own action rather than the submitter's formaction override, so a boosted click there would always have posted to the wrong endpoint regardless of the Alpine conflict. Also: - A reason for Out is now mandatory, not just captured -- empty and punctuation-only "answers" (a bare ".", "-", "??") are rejected server-side (the authoritative check) with textarea required/minlength as a client-side nudge on top. - Maybe can now carry an optional reason too, visible to the same audience as Out's (this member/family, and Coach mode's bench attendance) -- one shared reason form in event_detail.html's per-person row, its hidden status input following whichever of Maybe/Out was tapped. - The 3-way In/Maybe/Out row (and the 2-way hero In/Out) now use min-w-0 on every button so flex-1 actually splits the row evenly -- a longer label's own intrinsic width was winning it a bigger share otherwise. - HomeView's "Needs your answer" list now excludes events whose registration deadline has already passed -- replying is no longer possible there (same rule EventDetailView.post already enforces), so it doesn't belong in a "still needs a reply" list. hero_attendance is unaffected -- it always shows the true next event, falling back to a read-only pill once its own deadline closes. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01ECGMEwrc2k4D8VQuwjstj9 --- mobile/templates/mobile/_hero_rsvp.html | 20 +++-- .../templates/mobile/_notification_row.html | 2 +- .../mobile/calendar_feed_settings.html | 2 +- mobile/templates/mobile/coach/add_player.html | 2 +- mobile/templates/mobile/coach/attendance.html | 5 +- mobile/templates/mobile/coach/event_form.html | 2 +- mobile/templates/mobile/coach/lineup.html | 10 ++- mobile/templates/mobile/coach/news_form.html | 2 +- mobile/templates/mobile/coach/today.html | 9 +-- mobile/templates/mobile/edit_profile.html | 2 +- mobile/templates/mobile/event_detail.html | 40 ++++++---- mobile/templates/mobile/notifications.html | 4 +- mobile/tests.py | 74 ++++++++++++++++++- mobile/views.py | 35 +++++++-- 14 files changed, 160 insertions(+), 49 deletions(-) diff --git a/mobile/templates/mobile/_hero_rsvp.html b/mobile/templates/mobile/_hero_rsvp.html index ddefe6f..9b2909b 100644 --- a/mobile/templates/mobile/_hero_rsvp.html +++ b/mobile/templates/mobile/_hero_rsvp.html @@ -9,10 +9,14 @@ bench attendance (mobile/templates/mobile/coach/attendance.html) is the entire access story, no extra permission check needed. - Expects ``hero_attendance`` in scope. ``cross_shell`` (optional, truthy) - adds hx-boost="false" to both forms -- pass this when included from Coach - mode, since mobile:event_detail is a Member-shell view (see coach/ - today.html's own comment on why that matters). + Expects ``hero_attendance`` in scope. ``cross_shell`` no longer changes + anything here (kept as a no-op param for call-site compatibility) -- both + forms are unconditionally hx-boost="false" regardless of shell. Boosting + a form that Alpine also owns (x-show toggling between this pair of + sibling forms) had htmx and Alpine both trying to intercept the same + submit, which was breaking both In and Out. A real navigation for a + write action is a fine, standard trade -- hx-boost's value is in link-to- + link browsing, not swallowing every POST on the page. Neither button is colour-coded (no green "In") -- nobody has answered yet at this point (that's the whole reason for the "are you in?" prompt), so @@ -25,7 +29,7 @@ {% endcomment %}
- + {% csrf_token %} @@ -34,12 +38,12 @@
- + {% csrf_token %} - - + +
diff --git a/mobile/templates/mobile/_notification_row.html b/mobile/templates/mobile/_notification_row.html index d9c93d6..a1b48ef 100644 --- a/mobile/templates/mobile/_notification_row.html +++ b/mobile/templates/mobile/_notification_row.html @@ -10,7 +10,7 @@ treatment as management/templates/management/home.html's own notifications card. {% endcomment %} - + {% csrf_token %} diff --git a/mobile/templates/mobile/calendar_feed_settings.html b/mobile/templates/mobile/calendar_feed_settings.html index 07dfa48..be15d50 100644 --- a/mobile/templates/mobile/calendar_feed_settings.html +++ b/mobile/templates/mobile/calendar_feed_settings.html @@ -39,7 +39,7 @@
{% trans "Reset link" %}

{% trans "If you've shared this link and want to take it back, reset it -- the old one stops working immediately and you'll need to re-subscribe with the new one." %}

- + {% csrf_token %} diff --git a/mobile/templates/mobile/coach/add_player.html b/mobile/templates/mobile/coach/add_player.html index 0978349..1870ac2 100644 --- a/mobile/templates/mobile/coach/add_player.html +++ b/mobile/templates/mobile/coach/add_player.html @@ -28,7 +28,7 @@
-
+ {% csrf_token %}
{% for candidate in candidates %} diff --git a/mobile/templates/mobile/coach/attendance.html b/mobile/templates/mobile/coach/attendance.html index bf16ecc..3678b3b 100644 --- a/mobile/templates/mobile/coach/attendance.html +++ b/mobile/templates/mobile/coach/attendance.html @@ -36,7 +36,7 @@
- + {% csrf_token %}
{% for row in rows %} @@ -45,7 +45,8 @@
{{ row.member.get_full_name }}
{{ row.membership.position|default:"—" }}
- {% if row.status == "absent" and row.note %} + {% if row.note %} + {# Only ever set alongside absent/maybe -- EventDetailView.post clears it for present. #}
“{{ row.note }}”
{% endif %}
diff --git a/mobile/templates/mobile/coach/event_form.html b/mobile/templates/mobile/coach/event_form.html index 85f06ec..e8e5d4e 100644 --- a/mobile/templates/mobile/coach/event_form.html +++ b/mobile/templates/mobile/coach/event_form.html @@ -24,7 +24,7 @@
{% endif %} - + {% csrf_token %}
diff --git a/mobile/templates/mobile/coach/lineup.html b/mobile/templates/mobile/coach/lineup.html index f6b5723..9dfb25a 100644 --- a/mobile/templates/mobile/coach/lineup.html +++ b/mobile/templates/mobile/coach/lineup.html @@ -11,6 +11,12 @@ throughout rather than overriding the shared .coach-sheet's own light background -- a real per-screen shell hook is more infrastructure than one screen justifies. + + Both forms are hx-boost="false" -- the first has three submit buttons + sharing one via formaction overrides (Save/​+Add slot/​+Add line), + and htmx's boost reads the form's own action rather than the actual + submitter's formaction override, so a boosted click would always post + to the wrong endpoint. A plain navigation sidesteps that entirely. {% endcomment %} {% block header_extra %} @@ -22,7 +28,7 @@ {% endblock header_extra %} {% block content %} - + {% csrf_token %}
{% for unit in units %} @@ -67,7 +73,7 @@ {% endif %} {% if can_manage_active_team and not lineup.published_at %} - + {% csrf_token %} diff --git a/mobile/templates/mobile/coach/news_form.html b/mobile/templates/mobile/coach/news_form.html index 658d507..d197c9a 100644 --- a/mobile/templates/mobile/coach/news_form.html +++ b/mobile/templates/mobile/coach/news_form.html @@ -24,7 +24,7 @@
{% endif %} -
+ {% csrf_token %}
diff --git a/mobile/templates/mobile/coach/today.html b/mobile/templates/mobile/coach/today.html index da05ef4..eb79492 100644 --- a/mobile/templates/mobile/coach/today.html +++ b/mobile/templates/mobile/coach/today.html @@ -80,14 +80,7 @@ {% if rsvp_closed %} {{ hero_attendance.get_status_display }} {% else %} - {% comment %} - hx-boost="false" (passed as cross_shell below) -- mobile:event_detail - is a Member-shell view (bg-paper, not this page's bg-ink); boosting - would swap its response into this page's without updating the - tag's own class, leaving the wrong background. See base.html's - own comment for the full reasoning. - {% endcomment %} - {% include "mobile/_hero_rsvp.html" with cross_shell=True %} + {% include "mobile/_hero_rsvp.html" %} {% endif %}
diff --git a/mobile/templates/mobile/edit_profile.html b/mobile/templates/mobile/edit_profile.html index 3562707..43b421d 100644 --- a/mobile/templates/mobile/edit_profile.html +++ b/mobile/templates/mobile/edit_profile.html @@ -12,7 +12,7 @@ {% endcomment %} {% block content %} - + {% csrf_token %}
diff --git a/mobile/templates/mobile/event_detail.html b/mobile/templates/mobile/event_detail.html index 23aca96..dc3fdb8 100644 --- a/mobile/templates/mobile/event_detail.html +++ b/mobile/templates/mobile/event_detail.html @@ -11,6 +11,12 @@ under a dark gradient -- individual events have no photo of their own, so this is the one club-wide stand-in -- falling back to a plain dark background when the club hasn't uploaded one. + + The per-person In/Maybe/Out forms below are hx-boost="false" -- Alpine + owns the toggle between this row's two sibling forms (buttons vs. the + Out reason prompt), and having htmx *also* intercept the submit broke + both. A real navigation for a write action is a fine trade; hx-boost's + value is in link-to-link browsing, not every POST on the page. {% endcomment %} {% block content %} @@ -82,40 +88,44 @@ {% trans "Maybe" as label_maybe %} {% trans "Out" as label_out %} {% with status=answer.attendance.status %} -
+
- + {% csrf_token %} -
- {% csrf_token %} - - - - -
- + +
-
+ {% comment %} + One shared reason form for both Maybe (optional) and Out + (mandatory -- enforced server-side, EventDetailView.post) -- + :value on the hidden status input follows whichever button + was tapped. No required/minlength attrs here: mixing that + with a dynamic per-status requirement got fiddly for what + the backend already enforces authoritatively regardless. + {% endcomment %} + {% csrf_token %} - + - + +
- +
- {% if status == "absent" and answer.attendance.note %} + {% if answer.attendance.note %} + {# Only ever set alongside absent/maybe -- EventDetailView.post clears it for present. #}

“{{ answer.attendance.note }}”

{% endif %} {% endwith %} diff --git a/mobile/templates/mobile/notifications.html b/mobile/templates/mobile/notifications.html index f180a5c..783a39d 100644 --- a/mobile/templates/mobile/notifications.html +++ b/mobile/templates/mobile/notifications.html @@ -28,7 +28,7 @@ {% if today or earlier_this_week or older %}
{% if unread_notification_count %} -
+ {% csrf_token %} @@ -36,7 +36,7 @@ {% else %} {% endif %} - + {% csrf_token %} diff --git a/mobile/tests.py b/mobile/tests.py index 35ecd7d..15cf068 100644 --- a/mobile/tests.py +++ b/mobile/tests.py @@ -289,6 +289,24 @@ class HomeViewTests(TestCase): needs_answer_events = {attendance.event for attendance in response.context["needs_answer"]} self.assertEqual(needs_answer_events, {awaiting, maybe}) + def test_needs_your_answer_excludes_events_with_a_closed_registration_deadline(self): + # A distinct, already-answered earlier event so it becomes the hero -- + # otherwise the closed-deadline event below would become the hero + # itself (still shown there, just read-only) rather than reaching + # needs_answer's own exclusion at all. + hero_event = self.make_event(title="Soonest", start=self.future) + Attendance.objects.create(event=hero_event, member=self.member, status=Attendance.AttendanceStatus.PRESENT) + closed = self.make_event(title="Deadline passed", start=self.future + datetime.timedelta(days=2), deadline=timezone.now() - datetime.timedelta(hours=1)) + open_deadline = self.make_event(title="Deadline still open", start=self.future + datetime.timedelta(days=3), deadline=timezone.now() + datetime.timedelta(hours=1)) + Attendance.objects.create(event=closed, member=self.member, status=Attendance.AttendanceStatus.NO_RESPONSE) + Attendance.objects.create(event=open_deadline, member=self.member, status=Attendance.AttendanceStatus.NO_RESPONSE) + self.client.force_login(self.user) + + response = self._get("home") + + needs_answer_events = {attendance.event for attendance in response.context["needs_answer"]} + self.assertEqual(needs_answer_events, {open_deadline}) + def test_needs_your_answer_is_capped_at_five_with_a_remaining_count(self): # A distinct, already-answered earlier event so it becomes the hero and # none of the seven "Practice N" events below get excluded as the hero. @@ -524,7 +542,7 @@ class EventDetailRsvpTests(TestCase): other_event = Event.objects.create(club=self.club, title="Away game", start=timezone.now() + datetime.timedelta(days=8)) self.client.force_login(self.user) - self._post(other_event, {"status": "absent"}) + self._post(other_event, {"status": "absent", "note": "Sick"}) self.assertEqual(Attendance.objects.get(event=other_event, member=self.member).status, Attendance.AttendanceStatus.ABSENT) @@ -565,6 +583,25 @@ class EventDetailRsvpTests(TestCase): self.attendance.refresh_from_db() self.assertEqual(self.attendance.status, Attendance.AttendanceStatus.MAYBE) + def test_posting_maybe_without_a_reason_is_allowed(self): + # Unlike Out, a reason is optional for Maybe -- no 400 without one. + self.client.force_login(self.user) + + response = self._post(self.event, {"status": "maybe"}) + + self.assertRedirects(response, reverse("mobile:home"), fetch_redirect_response=False) + self.attendance.refresh_from_db() + self.assertEqual(self.attendance.note, "") + + def test_posting_maybe_with_a_reason_stores_the_note(self): + self.client.force_login(self.user) + + self._post(self.event, {"status": "maybe", "note": "Might have to leave early"}) + + self.attendance.refresh_from_db() + self.assertEqual(self.attendance.status, Attendance.AttendanceStatus.MAYBE) + self.assertEqual(self.attendance.note, "Might have to leave early") + def test_posting_absent_with_a_reason_stores_the_note(self): self.client.force_login(self.user) @@ -602,6 +639,31 @@ class EventDetailRsvpTests(TestCase): self.attendance.refresh_from_db() self.assertEqual(self.attendance.note, "") + def test_absent_without_a_reason_is_rejected(self): + self.client.force_login(self.user) + + response = self._post(self.event, {"status": "absent"}) + + self.assertEqual(response.status_code, 400) + self.attendance.refresh_from_db() + self.assertEqual(self.attendance.status, Attendance.AttendanceStatus.NO_RESPONSE) + + def test_absent_with_a_whitespace_only_reason_is_rejected(self): + self.client.force_login(self.user) + + response = self._post(self.event, {"status": "absent", "note": " \n "}) + + self.assertEqual(response.status_code, 400) + + def test_absent_with_a_punctuation_only_reason_is_rejected(self): + self.client.force_login(self.user) + + response = self._post(self.event, {"status": "absent", "note": "..."}) + + self.assertEqual(response.status_code, 400) + self.attendance.refresh_from_db() + self.assertEqual(self.attendance.status, Attendance.AttendanceStatus.NO_RESPONSE) + def test_rejects_an_unknown_status_value(self): self.client.force_login(self.user) @@ -2158,6 +2220,16 @@ class CoachAttendanceViewTests(TestCase): self.assertContains(response, "Anna Player") self.assertContains(response, "9") + def test_shows_a_maybe_reason_alongside_an_absent_one(self): + self.attendance.status = Attendance.AttendanceStatus.MAYBE + self.attendance.note = "Might be a few minutes late" + self.attendance.save() + self.client.force_login(self.user) + + response = self._get() + + self.assertContains(response, "Might be a few minutes late") + def test_save_records_check_ins_via_record_check_in(self): self.client.force_login(self.user) diff --git a/mobile/views.py b/mobile/views.py index 43e06cb..4823c5d 100644 --- a/mobile/views.py +++ b/mobile/views.py @@ -221,7 +221,16 @@ class HomeView(PersonScopeMixin, LoginRequiredMixin, TemplateView): deadline = hero_attendance.event.deadline rsvp_closed = deadline is not None and deadline < now - needs_answer_qs = upcoming.filter(status__in=[Attendance.AttendanceStatus.NO_RESPONSE, Attendance.AttendanceStatus.MAYBE]).order_by("event__start") + # Deadline already passed -> replying is no longer possible (see + # EventDetailView.post's own deadline check), so it doesn't belong + # in a "still needs a reply" list -- unlike hero_attendance above, + # which always shows the true next event regardless of RSVP state + # and falls back to a read-only pill once its own deadline closes. + needs_answer_qs = ( + upcoming.filter(status__in=[Attendance.AttendanceStatus.NO_RESPONSE, Attendance.AttendanceStatus.MAYBE]) + .filter(Q(event__deadline__isnull=True) | Q(event__deadline__gte=now)) + .order_by("event__start") + ) if hero_attendance is not None: needs_answer_qs = needs_answer_qs.exclude(pk=hero_attendance.pk) needs_answer_total = needs_answer_qs.count() @@ -444,14 +453,30 @@ class EventDetailView(PersonScopeMixin, LoginRequiredMixin, TemplateView): if event.deadline is not None and event.deadline < timezone.now(): return HttpResponseBadRequest(_("Replies are closed for this event.")) - # A reason is only ever meaningful attached to "Out" -- clearing it the - # moment someone flips back to In/Maybe avoids a stale "sick" note - # hanging around under an answer it no longer explains. Private by + # A reason is only ever meaningful attached to Out/Maybe -- clearing it + # the moment someone flips to In avoids a stale "sick" note hanging + # around under an answer it no longer explains. Private by # construction, not by a visibility flag: nothing renders another # member's own note anywhere -- only this member/family's own screens # (event_detail's "Your answers") and Coach mode's bench attendance # (mobile/templates/mobile/coach/attendance.html) ever read it. - note = request.POST.get("note", "").strip() if status == Attendance.AttendanceStatus.ABSENT else "" + note = "" + if status == Attendance.AttendanceStatus.ABSENT: + note = request.POST.get("note", "").strip() + # Rejects blank and punctuation-only "answers" (a bare ".", "-", + # "??") -- mandatory for Out specifically, unlike Maybe below. + # Backend-scoped, not the pretty inline-error UX this codebase + # gives ModelForm submissions elsewhere -- matches this view's + # own existing style (see "Unknown RSVP status"/"Replies are + # closed" above, both plain 400s a normal user should never + # actually see, since the template only ever offers Out through + # the reason form to begin with). + if not any(char.isalnum() for char in note): + return HttpResponseBadRequest(_("Please enter a reason.")) + elif status == Attendance.AttendanceStatus.MAYBE: + # Optional here -- Maybe doesn't owe anyone an explanation the way + # a firm no does, but the same field carries it if given one. + note = request.POST.get("note", "").strip() Attendance.objects.update_or_create(event=event, member=member, defaults={"status": status, "note": note}) if request.POST.get("next") == "event_detail":