From c4f0ec71c1fe869d88367099716d558f5f335043 Mon Sep 17 00:00:00 2001 From: Bernard Siebens Date: Tue, 11 Aug 2026 23:52:50 +0200 Subject: [PATCH] Season-scope claim history, self-service second-child claims, redesign approval screen Signed-in parents get their details locked and pre-filled on the claim form instead of retyped; approving links to their existing user and merges into their existing family instead of creating a duplicate. The approval screen is now a card grid with a searchable, pre-selected child dropdown and a reason modal for rejection. The "already dealt with" history is scoped to the current season. Co-Authored-By: Claude Sonnet 5 --- .../management/parent_claim_list.html | 104 +++++++------ management/tests.py | 145 ++++++++++++++++++ management/views.py | 14 +- members/forms.py | 39 ++++- .../0006_parentclaim_submitted_by_user.py | 21 +++ members/models.py | 9 ++ members/services/claims.py | 45 +++++- members/services/family.py | 18 ++- members/templates/members/parent_claim.html | 16 +- members/tests.py | 69 +++++++++ members/views.py | 31 +++- 11 files changed, 445 insertions(+), 66 deletions(-) create mode 100644 members/migrations/0006_parentclaim_submitted_by_user.py diff --git a/management/templates/management/parent_claim_list.html b/management/templates/management/parent_claim_list.html index 96819ba..db5719e 100644 --- a/management/templates/management/parent_claim_list.html +++ b/management/templates/management/parent_claim_list.html @@ -1,5 +1,5 @@ {% extends "management/base.html" %} -{% load i18n lucide ui %} +{% load i18n lucide static ui %} {% block heading %}{% trans "Parent claims" %}{% endblock heading %} {% block subheading %}{% trans "Parents asking to be linked to a child the club already has on file." %}{% endblock subheading %} @@ -12,56 +12,64 @@ {% blocktrans %}Check each request against your own records before approving. The shortlist only ever contains children who have nobody on file, so approving can never re-parent a child who already has one. An approved parent is added as a guardian: they get the login, but owe no fee and are not counted as a member.{% endblocktrans %}

- {% for claim in pending %} -
-
-
-
{% trans "Parent says they are" %}
-
{{ claim.parent_name }}
-
{{ claim.parent_email }}
-
-
-
{% trans "Parent of" %}
-
{{ claim.claimed_child_name }}
-
{{ claim.child_date_of_birth|date:"j F Y" }}
+
+ {% for claim in pending %} +
+
+
+
{% trans "Parent says they are" %}
+
{{ claim.parent_name }}
+
{{ claim.parent_email }}
+
+
+
{% trans "Parent of" %}
+
{{ claim.claimed_child_name }}
+
{{ claim.child_date_of_birth|date:"j F Y" }}
+
+ + {% comment %} + size="small" + show_label=False on the dropdown: {% form_field %} + defaults to daisyUI's medium control height, taller than the btn-sm + used by Approve/Reject below -- without it the dropdown stands out + against the buttons either side of it. + + justify-between (rather than everything in one flex-wrap run) keeps + Approve and Reject apart even as the row wraps on a narrow card -- + Approve stays pinned left, Reject right, so the two are never one + stray click apart the way adjacent buttons would be. + {% endcomment %} +
+ {% if claim.has_candidates %} +
+ {% csrf_token %} +
{% form_field claim.review_form.child size="small" show_label=False %}
+ +
+ {% else %} +

{% trans "No child without a parent matches this. Check the spelling and the date of birth with them before rejecting." %}

+ {% endif %} + + +
- - {% comment %} - size="small" + show_label=False on the dropdown: {% form_field %} - defaults to daisyUI's medium control height, taller than the btn-sm/ - input-sm used by everything else in this row -- without it the - dropdown stands out against the buttons either side of it. - - justify-between (rather than everything in one flex-wrap run) keeps - Approve and Reject apart even as the row wraps on a narrow screen -- - Approve stays pinned left, Reject right, so the two are never one - stray click apart the way adjacent buttons would be. - {% endcomment %} -
- {% if claim.has_candidates %} -
- {% csrf_token %} -
{% form_field claim.review_form.child size="small" show_label=False %}
- -
- {% else %} -

{% trans "No child without a parent matches this. Check the spelling and the date of birth with them before rejecting." %}

- {% endif %} - -
- {% csrf_token %} - - -
-
-
- {% empty %} -

{% trans "Nothing waiting." %}

- {% endfor %} + {% empty %} +

{% trans "Nothing waiting." %}

+ {% endfor %} +
+ {% trans "Reject claim" as reject_claim_title %} + {% trans "Reject" as reject_claim_submit_label %} + {% for claim in pending %} + {% url 'management:parent_claim_reject' claim.pk as reject_claim_url %} + {% blocktrans with name=claim.parent_name asvar reject_claim_blurb %}Why is {{ name }}'s claim being rejected? Recorded in the club's own history, not sent to them.{% endblocktrans %} + {% include "controlpanel/_modal_form.html" with modal_id=claim.pk|dom_id:"claim_reject_modal" title=reject_claim_title form=claim.reject_form action_url=reject_claim_url submit_label=reject_claim_submit_label submit_icon="x" blurb=reject_claim_blurb %} + {% endfor %} +

{% lucide "user-search" size=18 %} {% trans "Children with nobody on file" %}

@@ -117,3 +125,7 @@
{% endif %} {% endblock panel %} + +{% block extra_body %} + +{% endblock extra_body %} diff --git a/management/tests.py b/management/tests.py index b883c3b..7ab22ba 100644 --- a/management/tests.py +++ b/management/tests.py @@ -1576,6 +1576,151 @@ class ParentClaimViewTests(ManagementTestBase): self.assertContains(response, "Jamie Doe") + def test_the_child_dropdown_is_searchable_and_the_top_match_is_preselected(self): + self.submit() + self.client.force_login(self.admin_user) + + response = self.club_get("parent_claim_list") + + self.assertContains(response, 'data-searchable="true"') + html = response.content.decode() + select_tag = html[html.index("", html.index(" alongside it rather than merging with it. - child = forms.ModelChoiceField(queryset=None, label=_("Link to"), widget=forms.Select()) + # data-searchable matches every other member-picker in the app (e.g. + # TeamMembershipForm.member) -- useful once a club has many candidates. + child = forms.ModelChoiceField( + queryset=None, + label=_("Link to"), + widget=forms.Select(attrs={"data-searchable": "true", "data-search-placeholder": _("Type a name to search...")}), + ) def __init__(self, *args, candidates=None, **kwargs): super().__init__(*args, **kwargs) + if candidates is None: + candidates = Member.objects.none() self.fields["child"].queryset = candidates self.fields["child"].label_from_instance = lambda child: f"{child} ({child.date_of_birth:%d %b %Y})" if child.date_of_birth else str(child) + # Pre-select the top match rather than leaving the empty placeholder as + # the genuinely-selected option -- see management.views.ParentClaimListView. + top_match = candidates.first() + if top_match is not None: + self.fields["child"].initial = top_match.pk + + +class ClaimRejectForm(forms.Form): + """The optional reason recorded when an admin rejects a claim -- see + members.services.claims.reject_claim. Shown in a modal rather than an + inline row input so a real reason isn't fighting for space next to the + Approve/Reject buttons.""" + + note = forms.CharField(label=_("Reason"), required=False, widget=forms.Textarea(attrs={"rows": 2}), help_text=_("Recorded in the club's own history -- not sent to the parent.")) diff --git a/members/migrations/0006_parentclaim_submitted_by_user.py b/members/migrations/0006_parentclaim_submitted_by_user.py new file mode 100644 index 0000000..24785da --- /dev/null +++ b/members/migrations/0006_parentclaim_submitted_by_user.py @@ -0,0 +1,21 @@ +# Generated by Django 6.0.6 on 2026-08-11 21:39 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('members', '0005_parent_claim'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.AddField( + model_name='parentclaim', + name='submitted_by_user', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='submitted_parent_claims', to=settings.AUTH_USER_MODEL, verbose_name='submitted by'), + ), + ] diff --git a/members/models.py b/members/models.py index 92e3e60..e3483b7 100644 --- a/members/models.py +++ b/members/models.py @@ -131,6 +131,15 @@ class ParentClaim(ClubScopedModel): parent_last_name = models.CharField(_("parent last name"), max_length=150) parent_email = models.EmailField(_("parent email")) + # The signed-in account that submitted this claim, if any -- e.g. a parent + # already linked to one child, claiming a second. Distinct from parent_email + # above: that stays free text kept verbatim as the evidence an admin judged, + # while this is the authoritative "is this actually you" signal, set from + # request.user and never taken from anything the client could fake. Null for + # an anonymous public submission, which is the common case for a family's + # first claim. + submitted_by_user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.SET_NULL, null=True, blank=True, related_name="submitted_parent_claims", verbose_name=_("submitted by")) + # What the parent typed, kept verbatim even after the claim is matched -- it is # the evidence the admin judged, and a later dispute needs to see it unchanged. child_first_name = models.CharField(_("child first name"), max_length=150) diff --git a/members/services/claims.py b/members/services/claims.py index 921e69b..1bb4230 100644 --- a/members/services/claims.py +++ b/members/services/claims.py @@ -65,13 +65,18 @@ def children_awaiting_a_parent(club): ).distinct() -def submit_claim(club, *, parent_first_name, parent_last_name, parent_email, child_first_name, child_last_name, child_date_of_birth): +def submit_claim(club, *, parent_first_name, parent_last_name, parent_email, child_first_name, child_last_name, child_date_of_birth, submitted_by_user=None): """Record a claim from the public form. Deliberately does not check whether the child exists: the form is public, so telling the submitter either way would turn it into a way to enumerate the club's children. Everything is - judged by a human afterwards.""" + judged by a human afterwards. + + ``submitted_by_user`` is the signed-in account submitting a second (or + later) claim -- e.g. a parent already linked to one child, claiming a + sibling. ``None`` for the common case of an anonymous public submission.""" return ParentClaim.objects.create( club=club, + submitted_by_user=submitted_by_user if submitted_by_user is not None and submitted_by_user.is_authenticated else None, parent_first_name=parent_first_name.strip(), parent_last_name=parent_last_name.strip(), parent_email=parent_email.strip().lower(), @@ -117,17 +122,47 @@ def approve_claim(claim, *, child, season, reviewed_by=None): if not claim.is_pending: raise ClaimError("This claim has already been dealt with.") - family = child.family_memberships.first() - if family is None: + child_membership = child.family_memberships.first() + if child_membership is None: raise ClaimError("That child is not in a family, so there is nothing to join.") + child_family = child_membership.family + + # A signed-in submitter's account is authoritative for who the parent is -- + # resolved straight off the FK, never re-derived from the free-text + # parent_email, so a second claim from the same parent can never fork off a + # duplicate User/Member even if the typed email drifted from their login one. + parent = Member.objects.filter(user=claim.submitted_by_user).first() if claim.submitted_by_user_id else None + existing_membership = parent.family_memberships.first() if parent is not None else None + + if existing_membership is not None and existing_membership.family_id != child_family.pk: + # The parent already has a household on file -- most often because this + # is a second (or third) child of theirs being claimed. A parent who + # already exists as a Member is the anchor for "one household" from + # then on: the child's own solo family (created for them on import, see + # families_awaiting_a_parent above) merges into the parent's existing + # family, rather than the two staying split across separate Family + # rows or the parent forking into a fresh family alongside their other + # child. The child's now-empty solo family is cleaned up the same way + # members.services.family.detach_from_family already does elsewhere. + family = existing_membership.family + child_membership.family = family + child_membership.save(update_fields=["family"]) + if not child_family.memberships.exists(): + child_family.delete() + else: + # No existing household to anchor to (a brand new parent, or one whose + # only family link is this very child) -- fall back to the original + # behaviour of joining the child's own family. + family = child_family add_parent_to_family( claim.club, season, - family.family, + family, email=claim.parent_email, first_name=claim.parent_first_name, last_name=claim.parent_last_name, + parent=parent, ) claim.status = ParentClaim.Status.APPROVED diff --git a/members/services/family.py b/members/services/family.py index de11ecf..1b9b948 100644 --- a/members/services/family.py +++ b/members/services/family.py @@ -133,12 +133,20 @@ def add_child_to_family(club, season, family, *, first_name, last_name, date_of_ @transaction.atomic -def add_parent_to_family(club, season, family, *, email, first_name="", last_name="", parent_is_member=False): +def add_parent_to_family(club, season, family, *, email="", first_name="", last_name="", parent_is_member=False, parent=None): """A family that needs one more parent/guardian registered. A guardian - unless ``parent_is_member`` says they belong to the club in their own right.""" - parent = get_or_create_login_member(email, first_name, last_name) - # get_or_create, not create: re-adding an email already on this family (a typo'd - # re-submit, say) must not trip unique_member_per_family. + unless ``parent_is_member`` says they belong to the club in their own right. + + ``parent`` lets a caller that already knows the Member (e.g. + members.services.claims.approve_claim, once a claim carries a signed-in + submitter) attach them directly instead of resolving ``email`` again -- + authoritative when the caller has it, and the only way to guarantee no + second User/Member is ever created for the same person. + """ + if parent is None: + parent = get_or_create_login_member(email, first_name, last_name) + # get_or_create, not create: re-adding a parent already on this family (a typo'd + # re-submit, or a second claim that merged into it) must not trip unique_member_per_family. FamilyMembership.objects.get_or_create(family=family, member=parent, defaults={"role": FamilyMembership.FamilyRole.PARENT}) _enrol(club, season, parent, kind=ClubMembership.Kind.MEMBER if parent_is_member else ClubMembership.Kind.GUARDIAN) diff --git a/members/templates/members/parent_claim.html b/members/templates/members/parent_claim.html index 3332856..709b1de 100644 --- a/members/templates/members/parent_claim.html +++ b/members/templates/members/parent_claim.html @@ -20,11 +20,17 @@ {% endfor %}

{% trans "About you" %}

-
- {% form_field form.parent_first_name %} - {% form_field form.parent_last_name %} -
- {% form_field form.parent_email %} + {% if locked_member %} +

+ {% blocktrans with name=locked_member.get_full_name email=locked_member.contact_email %}Submitting as: {{ name }} ({{ email }}){% endblocktrans %} +

+ {% else %} +
+ {% form_field form.parent_first_name %} + {% form_field form.parent_last_name %} +
+ {% form_field form.parent_email %} + {% endif %}

{% trans "About your child" %}

diff --git a/members/tests.py b/members/tests.py index 136bdda..ac6fd28 100644 --- a/members/tests.py +++ b/members/tests.py @@ -806,3 +806,72 @@ class ParentClaimTests(TestCase): with self.assertRaises(ClaimError): approve_claim(claim, child=self.child, season=self.season) + + def test_submit_claim_records_the_signed_in_submitter(self): + user = User.objects.create_user(email="taylor.doe@example.com", password="x") + + claim = self.make_claim(submitted_by_user=user) + + self.assertEqual(claim.submitted_by_user, user) + + def test_submit_claim_leaves_the_submitter_blank_for_an_anonymous_submission(self): + claim = self.make_claim() + + self.assertIsNone(claim.submitted_by_user) + + def test_approving_a_second_claim_from_a_known_parent_merges_into_their_existing_family(self): + # A parent claiming a second (or third) child of theirs: the child's own + # solo family (created for them on import) must merge into the family + # the parent already belongs to, not sit alongside it in a second row. + first_claim = self.make_claim() + approve_claim(first_claim, child=self.child, season=self.season) + parent_user = User.objects.get(email="taylor.doe@example.com") + parent = Member.objects.get(user=parent_user) + + second_child = Member.objects.create(first_name="Robin", last_name="Doe", date_of_birth=datetime.date(2016, 5, 1)) + ClubMembership.objects.create(club=self.club, member=second_child, season=self.season, status=ClubMembership.StatusChoices.ACTIVE) + second_family = Family.objects.create() + FamilyMembership.objects.create(family=second_family, member=second_child, role=FamilyMembership.FamilyRole.CHILD) + second_claim = self.make_claim( + child_first_name="Robin", + child_last_name="Doe", + child_date_of_birth=datetime.date(2016, 5, 1), + submitted_by_user=parent_user, + ) + + approve_claim(second_claim, child=second_child, season=self.season) + + # Exactly one account and one household with both children. + self.assertEqual(User.objects.filter(email="taylor.doe@example.com").count(), 1) + self.assertEqual(Member.objects.filter(user=parent_user).count(), 1) + family = FamilyMembership.objects.get(member=parent).family + self.assertCountEqual(family.children, [self.child, second_child]) + self.assertFalse(Family.objects.filter(pk=second_family.pk).exists()) + # Still a guardian on the second child's enrolment too, not a member. + self.assertEqual(ClubMembership.objects.get(club=self.club, member=parent).kind, ClubMembership.Kind.GUARDIAN) + + def test_approving_falls_back_to_the_childs_family_when_the_parent_has_none_yet(self): + # A known account with no family link at all (e.g. login granted without + # ever being attached to a family) -- nothing to anchor to, so this + # behaves exactly like the original, family-less flow. + user = User.objects.create_user(email="taylor.doe@example.com", password="x") + parent = Member.objects.create(user=user, first_name="Taylor", last_name="Doe") + claim = self.make_claim(submitted_by_user=user) + + approve_claim(claim, child=self.child, season=self.season) + + self.assertEqual(FamilyMembership.objects.get(member=parent).family, self.family) + self.assertEqual(Member.objects.filter(user=user).count(), 1) + + def test_approving_uses_the_authenticated_members_account_even_if_the_typed_email_differs(self): + # submitted_by_user is authoritative -- a stale or mistyped parent_email + # must never fork off a second User/Member for someone already known. + user = User.objects.create_user(email="real.taylor@example.com", password="x") + parent = Member.objects.create(user=user, first_name="Taylor", last_name="Doe") + claim = self.make_claim(parent_email="typo.taylor@example.com", submitted_by_user=user) + + approve_claim(claim, child=self.child, season=self.season) + + self.assertEqual(Member.objects.filter(first_name="Taylor", last_name="Doe").count(), 1) + self.assertFalse(User.objects.filter(email="typo.taylor@example.com").exists()) + self.assertEqual(FamilyMembership.objects.get(member=parent).family, self.family) diff --git a/members/views.py b/members/views.py index c84984a..7cabb93 100644 --- a/members/views.py +++ b/members/views.py @@ -34,13 +34,42 @@ class ParentClaimView(ClubScopedPublicMixin, FormView): It always reports the same thing back, whether or not the child was found: the response must not tell an anonymous submitter which children the club has. What actually happens next is decided by an admin. + + A signed-in parent claiming a second child (a sibling, most often) gets + their "about you" fields locked to read-only text instead of blank inputs + -- typing their name/email again risks a typo forking off a second + User/Member instead of reusing theirs, and approval also uses this to + merge the new child into the family they and their first child already + belong to (see members.services.claims.approve_claim). """ template_name = "members/parent_claim.html" form_class = ParentClaimForm + def get_member(self): + if not self.request.user.is_authenticated: + return None + return Member.objects.filter(user=self.request.user).first() + + def get_form_kwargs(self): + kwargs = super().get_form_kwargs() + kwargs["lock_parent_fields"] = self.get_member() is not None + return kwargs + + def get_context_data(self, **kwargs): + return super().get_context_data(locked_member=self.get_member(), **kwargs) + def form_valid(self, form): - submit_claim(self.request.club, **form.cleaned_data) + data = dict(form.cleaned_data) + member = self.get_member() + if member is not None: + # Never trust the client for this -- the fields aren't even in the + # submitted form when locked. Pulled straight from the account + # that's actually signed in. + data["parent_first_name"] = member.first_name + data["parent_last_name"] = member.last_name + data["parent_email"] = member.contact_email + submit_claim(self.request.club, submitted_by_user=self.request.user if self.request.user.is_authenticated else None, **data) club = self.request.club title = _("Request received")