Email a set-password link on claim approval, flash identically either way
Two additions to the parent-claim flow: Club.contact_email (set from the control panel, next to legal_name), and an email sent when an admin approves a claim -- a real one-time set-password link built with allauth's own token generator, so it lands in the same flow the login page's own reset would send a parent to rather than a second, parallel one that could drift out of step with it. Never allowed to fail the approval: the family link and the guardian row are real either way, and a mail server being briefly unreachable must not cost a parent their place in the queue. The admin gets a distinct warning telling them the email didn't go and to have the parent use "Forgot your password?" instead. The public submission flash keeps the enumeration guarantee the claim form itself was built around: worded and timed identically whether or not a matching child was found, sent before any lookup happens at all, mentioning the club's contact email when the club has set one. A test compares the rendered flash across a matching and a non-matching submission byte for byte. One test-writing trap worth recording: assertRedirects follows the redirect itself by default, and its own probe GET consumed the one-shot flash message before a later explicit GET in the same test could see it -- fetch_redirect_response=False avoids the double-fetch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -450,6 +450,22 @@ ClubMembership(ClubScopedModel) # -> carries `club`
|
||||
afterwards; approving a claim is not the place to decide it. They then get a password-reset
|
||||
link and a minimal "my family" page (`members/views.py::MyFamilyView`) — the seam a real
|
||||
parent portal would grow from.
|
||||
- **`Club.contact_email`** *(built)* — the club's own public address, set from the control
|
||||
panel next to `legal_name`. Shown to the parent both in the submission flash and in the
|
||||
approval email, as somewhere to write if something's wrong — falls back to nothing shown at
|
||||
all when unset, same pattern as `legal_name`/`official_name`.
|
||||
- **The flash after submitting is worded and timed to reveal nothing.** Sent *before* any
|
||||
lookup happens, from a fixed string that never varies with whether a matching child was
|
||||
actually found (`members/views.py::ParentClaimView.form_valid`) — a message that differed
|
||||
would be exactly the enumeration channel free-text matching was built to avoid.
|
||||
- **Approval emails a real one-time set-password link**, built with allauth's own token
|
||||
generator (`default_token_generator`/`user_pk_to_url_str`) so it lands in the same flow the
|
||||
login page's own reset would send them to, rather than a second, parallel one that could
|
||||
drift out of step with it. Sending is never allowed to fail the approval — the family link
|
||||
and the guardian row are real either way, and a briefly unreachable mail server must not
|
||||
cost the parent their place in the queue; the admin sees a distinct warning message
|
||||
(`management/views.py::ParentClaimApproveView`) telling them the email didn't go and to have
|
||||
the parent use "Forgot your password?" instead.
|
||||
- **Why a field and not a separate model.** Everything that answers "is this person attached
|
||||
to this club" already reads through `ClubMembership` — tenancy scoping, group membership,
|
||||
the club-wide event audience — and a second kind of link would need a parallel path through
|
||||
|
||||
18
club/migrations/0024_club_contact_email.py
Normal file
18
club/migrations/0024_club_contact_email.py
Normal file
@@ -0,0 +1,18 @@
|
||||
# Generated by Django 6.0.6 on 2026-08-11 16:26
|
||||
|
||||
from django.db import migrations, models
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('club', '0023_backfill_guardian_memberships'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.AddField(
|
||||
model_name='club',
|
||||
name='contact_email',
|
||||
field=models.EmailField(blank=True, help_text="The club's public address, shown to people the club writes to or asks to get in touch -- e.g. a parent claiming a child. Falls back to nothing being shown at all, so it's worth setting.", max_length=254, verbose_name='contact email'),
|
||||
),
|
||||
]
|
||||
@@ -41,6 +41,11 @@ class Club(UUIDModel):
|
||||
name = models.CharField(_("name"), max_length=255)
|
||||
legal_name = models.CharField(_("legal name"), max_length=255, blank=True, help_text=_("Full registered name (e.g. including a legal form like VZW/ASBL), used on official documents. Falls back to club name if blank."))
|
||||
slug = models.SlugField(_("slug"), max_length=255, unique=True, blank=True, help_text=_("Drives subdomain / path resolution (e.g. ajax-united.rosterchief.app)."))
|
||||
contact_email = models.EmailField(
|
||||
_("contact email"),
|
||||
blank=True,
|
||||
help_text=_("The club's public address, shown to people the club writes to or asks to get in touch -- e.g. a parent claiming a child. Falls back to nothing being shown at all, so it's worth setting."),
|
||||
)
|
||||
|
||||
logo = models.FileField(
|
||||
_("logo"),
|
||||
|
||||
@@ -14,7 +14,7 @@ from .services.admins import find_member_by_email
|
||||
class ClubForm(forms.ModelForm):
|
||||
class Meta:
|
||||
model = Club
|
||||
fields = ["name", "legal_name", "slug", "sport_type", "logo", "primary_color", "secondary_color", "season_start", "season_duration_months"]
|
||||
fields = ["name", "legal_name", "contact_email", "slug", "sport_type", "logo", "primary_color", "secondary_color", "season_start", "season_duration_months"]
|
||||
help_texts = {"slug": _("Drives the club's subdomain. Left blank, it is derived from the name.")}
|
||||
# Deliberately a text input, not <input type="color">: a colour picker cannot
|
||||
# express "no colour" -- it would submit #000000 for every club that never
|
||||
|
||||
@@ -18,6 +18,7 @@
|
||||
<div class="grid grid-cols-1 md:grid-cols-2 gap-4">
|
||||
{% form_field form.name %}
|
||||
{% form_field form.legal_name %}
|
||||
{% form_field form.contact_email %}
|
||||
{% form_field form.slug %}
|
||||
{% form_field form.sport_type %}
|
||||
</div>
|
||||
|
||||
@@ -8,6 +8,7 @@ from unittest import mock
|
||||
import openpyxl
|
||||
from allauth.mfa.models import Authenticator
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.core import mail
|
||||
from django.core.cache import cache
|
||||
from django.core.files.uploadedfile import SimpleUploadedFile
|
||||
from django.template.loader import render_to_string
|
||||
@@ -1389,27 +1390,38 @@ class ParentClaimViewTests(ManagementTestBase):
|
||||
payload.update(overrides)
|
||||
return payload
|
||||
|
||||
def submit(self, **overrides):
|
||||
return self.client.post(reverse("members:parent_claim"), self.claim_payload(**overrides), HTTP_HOST="ajax-united.rosterchief.app")
|
||||
def submit(self, *, follow=False, **overrides):
|
||||
return self.client.post(reverse("members:parent_claim"), self.claim_payload(**overrides), HTTP_HOST="ajax-united.rosterchief.app", follow=follow)
|
||||
|
||||
def test_the_claim_form_is_reachable_without_signing_in(self):
|
||||
response = self.client.get(reverse("members:parent_claim"), HTTP_HOST="ajax-united.rosterchief.app")
|
||||
|
||||
self.assertEqual(response.status_code, 200)
|
||||
|
||||
def test_submitting_records_a_pending_claim(self):
|
||||
def test_submitting_records_a_pending_claim_and_redirects_back_with_a_flash(self):
|
||||
response = self.submit()
|
||||
|
||||
self.assertEqual(response.status_code, 200)
|
||||
# fetch_redirect_response=False: assertRedirects' own probe GET would
|
||||
# otherwise consume the one-shot flash before the assertion below gets to see it.
|
||||
self.assertRedirects(response, reverse("members:parent_claim"), fetch_redirect_response=False)
|
||||
self.assertEqual(ParentClaim.objects.filter(club=self.club, status=ParentClaim.Status.PENDING).count(), 1)
|
||||
page = self.client.get(reverse("members:parent_claim"), HTTP_HOST="ajax-united.rosterchief.app")
|
||||
self.assertContains(page, "Request received")
|
||||
|
||||
def test_an_unmatched_claim_looks_exactly_like_a_matched_one(self):
|
||||
def test_an_unmatched_claim_flashes_the_same_message_as_a_matched_one(self):
|
||||
# The page must not tell an anonymous submitter which children exist.
|
||||
matched = self.submit()
|
||||
unmatched = self.submit(child_first_name="Nobody", child_last_name="Here", parent_email="other@example.com")
|
||||
matched = self.submit(follow=True)
|
||||
unmatched = self.submit(child_first_name="Nobody", child_last_name="Here", parent_email="other@example.com", follow=True)
|
||||
|
||||
self.assertEqual(matched.status_code, unmatched.status_code)
|
||||
self.assertEqual(matched.content, unmatched.content)
|
||||
self.assertEqual([str(m) for m in matched.context["messages"]], [str(m) for m in unmatched.context["messages"]])
|
||||
|
||||
def test_the_flash_mentions_the_clubs_contact_email_when_set(self):
|
||||
self.club.contact_email = "info@ajax-united.example.com"
|
||||
self.club.save(update_fields=["contact_email"])
|
||||
|
||||
response = self.submit(follow=True)
|
||||
|
||||
self.assertContains(response, "info@ajax-united.example.com")
|
||||
|
||||
def test_submitting_creates_no_account(self):
|
||||
# A public form that made a User per submission would be a spam magnet;
|
||||
@@ -1454,6 +1466,49 @@ class ParentClaimViewTests(ManagementTestBase):
|
||||
claim.refresh_from_db()
|
||||
self.assertEqual(claim.status, ParentClaim.Status.APPROVED)
|
||||
|
||||
def test_approving_emails_the_parent_a_working_set_password_link(self):
|
||||
self.submit()
|
||||
claim = ParentClaim.objects.get(club=self.club)
|
||||
self.client.force_login(self.admin_user)
|
||||
|
||||
self.club_post("parent_claim_approve", {"child": str(self.child.pk)}, claim.pk)
|
||||
|
||||
self.assertEqual(len(mail.outbox), 1)
|
||||
sent = mail.outbox[0]
|
||||
self.assertEqual(sent.to, ["taylor.doe@example.com"])
|
||||
self.assertIn("Jamie", sent.body)
|
||||
|
||||
[reset_path] = [line for line in sent.body.splitlines() if "/accounts/password/reset/key/" in line]
|
||||
parent = Member.objects.get(user__email="taylor.doe@example.com")
|
||||
self.assertFalse(parent.user.has_usable_password())
|
||||
response = self.client.get(reset_path.strip(), HTTP_HOST="ajax-united.rosterchief.app", follow=True)
|
||||
self.assertEqual(response.status_code, 200)
|
||||
self.assertContains(response, "password1")
|
||||
|
||||
def test_the_email_mentions_the_clubs_contact_email_when_set(self):
|
||||
self.club.contact_email = "info@ajax-united.example.com"
|
||||
self.club.save(update_fields=["contact_email"])
|
||||
self.submit()
|
||||
claim = ParentClaim.objects.get(club=self.club)
|
||||
self.client.force_login(self.admin_user)
|
||||
|
||||
self.club_post("parent_claim_approve", {"child": str(self.child.pk)}, claim.pk)
|
||||
|
||||
self.assertIn("info@ajax-united.example.com", mail.outbox[0].body)
|
||||
|
||||
def test_a_send_failure_still_leaves_the_claim_linked(self):
|
||||
self.submit()
|
||||
claim = ParentClaim.objects.get(club=self.club)
|
||||
self.client.force_login(self.admin_user)
|
||||
|
||||
with mock.patch("members.services.claims.send_mail", side_effect=OSError("smtp down")):
|
||||
response = self.club_post("parent_claim_approve", {"child": str(self.child.pk)}, claim.pk)
|
||||
|
||||
self.assertRedirects(response, reverse("management:parent_claim_list"))
|
||||
claim.refresh_from_db()
|
||||
self.assertEqual(claim.status, ParentClaim.Status.APPROVED)
|
||||
self.assertTrue(Member.objects.filter(user__email="taylor.doe@example.com").exists())
|
||||
|
||||
def test_approving_without_choosing_a_child_changes_nothing(self):
|
||||
self.submit()
|
||||
claim = ParentClaim.objects.get(club=self.club)
|
||||
|
||||
@@ -40,7 +40,7 @@ from formbuilder.models import Form as FormBuilderForm
|
||||
from formbuilder.models import Submission
|
||||
from members.forms import ClaimReviewForm
|
||||
from members.models import Family, FamilyMembership, Group, GroupMembership, Member, ParentClaim
|
||||
from members.services.claims import ClaimError, approve_claim, children_awaiting_a_parent, reject_claim, suggested_children
|
||||
from members.services.claims import ClaimError, approve_claim, children_awaiting_a_parent, reject_claim, send_claim_approved_email, suggested_children
|
||||
from members.services.family import add_child_to_family, add_parent_to_family, attach_to_family, detach_from_family, get_or_create_login_user, grant_login, register_family
|
||||
from news.models import News, NewsPhoto
|
||||
from shop.models import Discount, Invoice, Order, Product
|
||||
@@ -1487,8 +1487,16 @@ class ParentClaimApproveView(ClubAdminRequiredMixin, View):
|
||||
except ClaimError as error:
|
||||
notify(request, f"e|{_('Could not approve')}|{error}")
|
||||
else:
|
||||
body = _("“%(parent)s” is now linked to %(child)s and can set up their login.") % {"parent": claim.parent_name, "child": form.cleaned_data["child"]}
|
||||
notify(request, f"s|{_('Claim approved')}|{body}")
|
||||
child = form.cleaned_data["child"]
|
||||
emailed = send_claim_approved_email(claim, child=child, request=request)
|
||||
if emailed:
|
||||
body = _("“%(parent)s” is now linked to %(child)s. They've been emailed a link to set their password.") % {"parent": claim.parent_name, "child": child}
|
||||
notify(request, f"s|{_('Claim approved')}|{body}")
|
||||
else:
|
||||
# The link is made either way -- say so plainly rather than letting
|
||||
# the club assume the parent has been told.
|
||||
body = _("“%(parent)s” is now linked to %(child)s, but the email could not be sent. Ask them to use “Forgot your password?” on the sign-in page.") % {"parent": claim.parent_name, "child": child}
|
||||
notify(request, f"w|{_('Claim approved, email not sent')}|{body}")
|
||||
return redirect("management:parent_claim_list")
|
||||
|
||||
|
||||
|
||||
@@ -12,14 +12,23 @@ must never confirm whether a given child exists -- so it takes free text and
|
||||
matches nothing itself.
|
||||
"""
|
||||
|
||||
from allauth.account.forms import default_token_generator
|
||||
from allauth.account.utils import user_pk_to_url_str
|
||||
from django.conf import settings
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.core.mail import send_mail
|
||||
from django.db import transaction
|
||||
from django.db.models import Exists, OuterRef, Q
|
||||
from django.template.loader import render_to_string
|
||||
from django.urls import reverse
|
||||
from django.utils import timezone
|
||||
|
||||
from club.models import ClubMembership
|
||||
from members.models import Family, FamilyMembership, Member, ParentClaim
|
||||
from members.services.family import add_parent_to_family
|
||||
|
||||
User = get_user_model()
|
||||
|
||||
#: Suggestions are ranked, never auto-applied -- an exact name-and-birthday match
|
||||
#: is still only a suggestion, because the whole point of the queue is that a
|
||||
#: human confirms it.
|
||||
@@ -139,3 +148,36 @@ def reject_claim(claim, *, reviewed_by=None, note=""):
|
||||
claim.note = note
|
||||
claim.save(update_fields=["status", "reviewed_by", "reviewed_at", "note"])
|
||||
return claim
|
||||
|
||||
|
||||
def send_claim_approved_email(claim, *, child, request=None):
|
||||
"""Tell the parent their account is ready, with a link that sets their password.
|
||||
|
||||
A real one-time link rather than "go to the reset page and type your email":
|
||||
the account was created for them with no usable password, so being told to
|
||||
"reset" something they never had reads as an error. Built with allauth's own
|
||||
token generator so it lands in the same flow the login page would send them
|
||||
to, rather than a second, parallel one that could drift out of step with it.
|
||||
|
||||
Never fatal: an approved claim is a real link in the database whether or not
|
||||
the mail leaves the building, and losing that link because a mail server was
|
||||
briefly unreachable would be far worse than a parent needing a nudge.
|
||||
"""
|
||||
user = User.objects.filter(email__iexact=claim.parent_email).first()
|
||||
if user is None:
|
||||
return False
|
||||
|
||||
path = reverse("account_reset_password_from_key", kwargs={"uidb36": user_pk_to_url_str(user), "key": default_token_generator.make_token(user)})
|
||||
set_password_url = request.build_absolute_uri(path) if request is not None else path
|
||||
|
||||
context = {"club": claim.club, "child": child, "parent_first_name": claim.parent_first_name, "set_password_url": set_password_url}
|
||||
subject = " ".join(render_to_string("members/email/claim_approved_subject.txt", context).split())
|
||||
body = render_to_string("members/email/claim_approved.txt", context).strip() + "\n"
|
||||
|
||||
try:
|
||||
send_mail(subject, body, settings.DEFAULT_FROM_EMAIL, [claim.parent_email], fail_silently=False)
|
||||
except OSError:
|
||||
# Anything the mail backend raises for an unreachable server or a refused
|
||||
# connection. The link stands; the club can resend from the queue.
|
||||
return False
|
||||
return True
|
||||
|
||||
13
members/templates/members/email/claim_approved.txt
Normal file
13
members/templates/members/email/claim_approved.txt
Normal file
@@ -0,0 +1,13 @@
|
||||
{% load i18n %}{% blocktrans with name=parent_first_name %}Hello {{ name }},{% endblocktrans %}
|
||||
|
||||
{% blocktrans with club=club.name child=child %}{{ club }} has confirmed that you're {{ child }}'s parent or guardian, and your account is ready.{% endblocktrans %}
|
||||
|
||||
{% trans "Set your password here:" %}
|
||||
{{ set_password_url }}
|
||||
{% blocktrans %}That link is for you alone — please don't forward it.{% endblocktrans %}
|
||||
|
||||
{% blocktrans %}Once you're signed in you'll see the children linked to you.{% endblocktrans %}
|
||||
{% if club.contact_email %}
|
||||
{% blocktrans with email=club.contact_email %}Something not right? Reply to this note or write to {{ email }}.{% endblocktrans %}
|
||||
{% endif %}
|
||||
{% blocktrans with club=club.name %}— {{ club }}{% endblocktrans %}
|
||||
@@ -0,0 +1 @@
|
||||
{% load i18n %}{% blocktrans with club=club.name %}Your {{ club }} account is ready{% endblocktrans %}
|
||||
@@ -1,20 +0,0 @@
|
||||
{% extends "_club_base.html" %}
|
||||
{% load i18n lucide %}
|
||||
|
||||
{% block head_title %}{% trans "Request sent" %}{% endblock head_title %}
|
||||
|
||||
{% block main %}
|
||||
{% comment %}
|
||||
Says the same thing whether or not the child was found: this page is public,
|
||||
so confirming a match would let anyone test which children the club has.
|
||||
{% endcomment %}
|
||||
<div class="flex justify-center">
|
||||
<div class="card w-full max-w-xl bg-base-100 shadow">
|
||||
<div class="card-body">
|
||||
<h1 class="card-title">{% lucide "circle-check" size=20 %} {% trans "Request sent" %}</h1>
|
||||
<p>{% blocktrans with club=club.name %}Thanks — {{ club }} will check this against their records.{% endblocktrans %}</p>
|
||||
<p class="text-sm opacity-70">{% blocktrans %}If it matches, you'll get an email with a link to set your password. If you don't hear anything, get in touch with the club directly.{% endblocktrans %}</p>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
{% endblock main %}
|
||||
@@ -6,9 +6,11 @@ staff, and ClubStaffRequiredMixin would (rightly) turn them away.
|
||||
|
||||
from django.contrib.auth.mixins import LoginRequiredMixin
|
||||
from django.http import Http404
|
||||
from django.shortcuts import render
|
||||
from django.shortcuts import redirect
|
||||
from django.utils.translation import gettext_lazy as _
|
||||
from django.views.generic import FormView, TemplateView
|
||||
|
||||
from controlpanel.messages import notify
|
||||
from members.forms import ParentClaimForm
|
||||
from members.models import FamilyMembership, Member
|
||||
from members.services.claims import submit_claim
|
||||
@@ -39,7 +41,17 @@ class ParentClaimView(ClubScopedPublicMixin, FormView):
|
||||
|
||||
def form_valid(self, form):
|
||||
submit_claim(self.request.club, **form.cleaned_data)
|
||||
return render(self.request, "members/parent_claim_submitted.html", {"club": self.request.club})
|
||||
|
||||
club = self.request.club
|
||||
title = _("Request received")
|
||||
body = _("%(club)s will check this against their records. If it matches, you'll get an email with a link to set your password.") % {"club": club.name}
|
||||
if club.contact_email:
|
||||
body += " " + _("Any questions, write to %(email)s.") % {"email": club.contact_email}
|
||||
# Worded identically whether or not that child exists, and sent before any
|
||||
# lookup happens at all -- a message that differed would tell an anonymous
|
||||
# submitter which children the club has.
|
||||
notify(self.request, f"s|{title}|{body}")
|
||||
return redirect("members:parent_claim")
|
||||
|
||||
|
||||
class MyFamilyView(ClubScopedPublicMixin, LoginRequiredMixin, TemplateView):
|
||||
|
||||
Reference in New Issue
Block a user