Remove OnboardingRequirement.order; restrict checklist actions to admin/MEMBER_ADMIN

Every active requirement blocks equally and there's no set order to complete
them in, so the configurable "order" field (and its ordering-by-number) is
gone -- requirements list alphabetically now, both in the admin UI and the
Onboarding requirements settings page.

Also closes a real permission gap found while checking this: marking a
checklist item complete, bypassing it, or reopening it (management/views.py's
MemberRequirementCompleteView/BypassView/IncompleteView) was open to *any*
staff member with page access, not just admin/MEMBER_ADMIN, despite the
member detail page's own Documents card implying otherwise. Switched all
three to MemberAdminRequiredMixin and hid the corresponding buttons/dialog
from anyone who can't use them. The Sign-up page's Bypass action was already
admin-only end to end (the whole page is ClubAdminRequiredMixin-gated), so
no change in practice there.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ECGMEwrc2k4D8VQuwjstj9
This commit is contained in:
2026-08-20 09:27:05 +02:00
parent 224a3fe4ed
commit 45380dc282
10 changed files with 101 additions and 45 deletions

View File

@@ -67,10 +67,10 @@ class ClubRoleAdmin(admin.ModelAdmin):
@admin.register(OnboardingRequirement) @admin.register(OnboardingRequirement)
class OnboardingRequirementAdmin(admin.ModelAdmin): class OnboardingRequirementAdmin(admin.ModelAdmin):
list_display = ["name", "club", "requires_document", "is_active", "order"] list_display = ["name", "club", "requires_document", "is_active"]
list_filter = ["club", "is_active", "requires_document"] list_filter = ["club", "is_active", "requires_document"]
search_fields = ["name", "club__name"] search_fields = ["name", "club__name"]
ordering = ["club", "order", "name"] ordering = ["club", "name"]
@admin.register(MemberRequirementStatus) @admin.register(MemberRequirementStatus)

View File

@@ -0,0 +1,21 @@
# Generated by Django 6.0.6 on 2026-08-20 07:21
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [
('club', '0027_club_website'),
]
operations = [
migrations.AlterModelOptions(
name='onboardingrequirement',
options={'ordering': ['name'], 'verbose_name': 'onboarding requirement', 'verbose_name_plural': 'onboarding requirements'},
),
migrations.RemoveField(
model_name='onboardingrequirement',
name='order',
),
]

View File

@@ -422,12 +422,14 @@ class OnboardingRequirement(ClubScopedModel):
requires_document = models.BooleanField(_("requires a document"), default=False, help_text=_("Staff can attach a file (e.g. the certificate itself) when marking this complete.")) requires_document = models.BooleanField(_("requires a document"), default=False, help_text=_("Staff can attach a file (e.g. the certificate itself) when marking this complete."))
blocked_event_kinds = models.JSONField(_("blocks selection for"), default=list, blank=True, help_text=_("Event kinds a member can't be invited to or selected for while this is open. Empty means purely informational.")) blocked_event_kinds = models.JSONField(_("blocks selection for"), default=list, blank=True, help_text=_("Event kinds a member can't be invited to or selected for while this is open. Empty means purely informational."))
is_active = models.BooleanField(_("active"), default=True, help_text=_("Inactive requirements no longer apply to new memberships, but existing statuses are kept.")) is_active = models.BooleanField(_("active"), default=True, help_text=_("Inactive requirements no longer apply to new memberships, but existing statuses are kept."))
order = models.PositiveIntegerField(_("order"), default=0, help_text=_("Lower numbers show first on the checklist."))
class Meta: class Meta:
verbose_name = _("onboarding requirement") verbose_name = _("onboarding requirement")
verbose_name_plural = _("onboarding requirements") verbose_name_plural = _("onboarding requirements")
ordering = ["order", "name"] # Alphabetical, not a configurable sequence: every active requirement
# blocks equally and there's no set order to complete them in, so
# ordering here is purely for a stable, predictable listing.
ordering = ["name"]
constraints = [ constraints = [
models.UniqueConstraint(fields=["club", "name"], name="unique_onboarding_requirement_name_per_club"), models.UniqueConstraint(fields=["club", "name"], name="unique_onboarding_requirement_name_per_club"),
] ]

View File

@@ -1817,8 +1817,8 @@ class OnboardingRequirementTests(TestCase):
club=cls.club, member=cls.member, season=cls.season, status=ClubMembership.StatusChoices.ACTIVE, fee_status=ClubMembership.FeeStatus.PAID club=cls.club, member=cls.member, season=cls.season, status=ClubMembership.StatusChoices.ACTIVE, fee_status=ClubMembership.FeeStatus.PAID
) )
cls.staff = get_user_model().objects.create_user(email="staff@example.com", password="pw-secret-123") cls.staff = get_user_model().objects.create_user(email="staff@example.com", password="pw-secret-123")
cls.photo = OnboardingRequirement.objects.create(club=cls.club, name="Photo", order=1) cls.photo = OnboardingRequirement.objects.create(club=cls.club, name="Photo")
cls.medical = OnboardingRequirement.objects.create(club=cls.club, name="Medical certificate", requires_document=True, order=2) cls.medical = OnboardingRequirement.objects.create(club=cls.club, name="Medical certificate", requires_document=True)
def test_a_membership_with_no_status_rows_has_every_requirement_open(self): def test_a_membership_with_no_status_rows_has_every_requirement_open(self):
self.assertEqual(self.membership.open_requirement_count, 2) self.assertEqual(self.membership.open_requirement_count, 2)

View File

@@ -904,7 +904,7 @@ class OnboardingRequirementForm(forms.ModelForm):
class Meta: class Meta:
model = OnboardingRequirement model = OnboardingRequirement
fields = ["name", "description", "requires_document", "blocked_event_kinds", "is_active", "order"] fields = ["name", "description", "requires_document", "blocked_event_kinds", "is_active"]
class RequirementCompletionForm(forms.Form): class RequirementCompletionForm(forms.Form):

View File

@@ -203,10 +203,12 @@
{% comment %} {% comment %}
Onboarding checklist -- club.services.onboarding.checklist_for, paired with Onboarding checklist -- club.services.onboarding.checklist_for, paired with
MemberRequirementStatus if one exists. Any staff can mark an item done or MemberRequirementStatus if one exists. Any staff can see it (same
reopen it, not just admins (same visibility as the rest of this page) -- visibility as the rest of this page), but only an admin/MEMBER_ADMIN can
see the module docstring on club/services/onboarding.py for why this stays mark an item done or reopen it (MemberAdminRequiredMixin on the three
separate from ClubMembership.status/fee_status. requirement views in management/views.py) -- see the module docstring on
club/services/onboarding.py for why this stays separate from
ClubMembership.status/fee_status.
{% endcomment %} {% endcomment %}
<div class="card card-body h-full"> <div class="card card-body h-full">
<h2 class="card-title">{% lucide "clipboard-check" size=18 %} {% trans "Documents" %}</h2> <h2 class="card-title">{% lucide "clipboard-check" size=18 %} {% trans "Documents" %}</h2>
@@ -238,6 +240,7 @@
<a class="mt-1 inline-flex items-center gap-1 text-sm link link-hover" href="{% url 'management:member_requirement_document' pk=member.pk requirement_pk=requirement.pk %}">{% lucide "download" size=13 %} {% trans "Download document" %}</a> <a class="mt-1 inline-flex items-center gap-1 text-sm link link-hover" href="{% url 'management:member_requirement_document' pk=member.pk requirement_pk=requirement.pk %}">{% lucide "download" size=13 %} {% trans "Download document" %}</a>
{% endif %} {% endif %}
</div> </div>
{% if can_manage_members %}
<div class="flex shrink-0 items-center gap-1.5"> <div class="flex shrink-0 items-center gap-1.5">
{% if status.is_complete %} {% if status.is_complete %}
<form method="post" action="{% url 'management:member_requirement_incomplete' pk=member.pk requirement_pk=requirement.pk %}"> <form method="post" action="{% url 'management:member_requirement_incomplete' pk=member.pk requirement_pk=requirement.pk %}">
@@ -248,6 +251,7 @@
<button class="btn btn-primary btn-xs gap-1" type="button" onclick="document.getElementById('{{ requirement.pk|dom_id:"requirement_complete_modal" }}').showModal()">{% lucide "check" size=12 %} {% trans "Mark complete" %}</button> <button class="btn btn-primary btn-xs gap-1" type="button" onclick="document.getElementById('{{ requirement.pk|dom_id:"requirement_complete_modal" }}').showModal()">{% lucide "check" size=12 %} {% trans "Mark complete" %}</button>
{% endif %} {% endif %}
</div> </div>
{% endif %}
</div> </div>
{% endfor %} {% endfor %}
</div> </div>
@@ -366,6 +370,7 @@
{% include "controlpanel/_modal_form.html" with modal_id="attach_family_modal" title=add_to_family_title form=attach_to_family_form action_url=attach_family_url submit_label=add_label submit_icon="user-plus" blurb=attach_family_blurb %} {% include "controlpanel/_modal_form.html" with modal_id="attach_family_modal" title=add_to_family_title form=attach_to_family_form action_url=attach_family_url submit_label=add_label submit_icon="user-plus" blurb=attach_family_blurb %}
{% endif %} {% endif %}
{% if can_manage_members %}
{% for requirement, status in checklist %} {% for requirement, status in checklist %}
{% if not status.is_complete %} {% if not status.is_complete %}
<dialog id="{{ requirement.pk|dom_id:"requirement_complete_modal" }}" class="modal"> <dialog id="{{ requirement.pk|dom_id:"requirement_complete_modal" }}" class="modal">
@@ -395,6 +400,7 @@
</dialog> </dialog>
{% endif %} {% endif %}
{% endfor %} {% endfor %}
{% endif %}
{% endblock panel %} {% endblock panel %}
{% block extra_body %} {% block extra_body %}

View File

@@ -3,11 +3,11 @@
{% comment %} {% comment %}
What this club requires from every member after they sign up or renew -- What this club requires from every member after they sign up or renew --
see club/models.py's OnboardingRequirement docstring. Order matters (shown see club/models.py's OnboardingRequirement docstring. Listed alphabetically,
as typed in `order`, ascending) -- it's the sequence staff see on a not in a configurable sequence: every active requirement blocks equally and
member's checklist. No dedicated mockup screen for this (new feature, not there's no set order to complete them in. No dedicated mockup screen for
in the original design file) -- extrapolates the card/table vocabulary this (new feature, not in the original design file) -- extrapolates the
used across Settings. card/table vocabulary used across Settings.
{% endcomment %} {% endcomment %}
{% block panel_title %}{% trans "Onboarding requirements" %}{% endblock panel_title %} {% block panel_title %}{% trans "Onboarding requirements" %}{% endblock panel_title %}

View File

@@ -6702,7 +6702,7 @@ class OnboardingRequirementManagementTests(ManagementTestBase):
self.client.force_login(self.admin_user) self.client.force_login(self.admin_user)
def test_an_admin_can_create_a_requirement(self): def test_an_admin_can_create_a_requirement(self):
response = self.club_post("onboarding_requirement_create", {"name": "Photo", "requires_document": "", "is_active": "on", "order": "1"}) response = self.club_post("onboarding_requirement_create", {"name": "Photo", "requires_document": "", "is_active": "on"})
self.assertRedirects(response, reverse("management:onboarding_requirement_list")) self.assertRedirects(response, reverse("management:onboarding_requirement_list"))
self.assertTrue(OnboardingRequirement.objects.filter(club=self.club, name="Photo").exists()) self.assertTrue(OnboardingRequirement.objects.filter(club=self.club, name="Photo").exists())
@@ -6713,7 +6713,7 @@ class OnboardingRequirementManagementTests(ManagementTestBase):
ClubMembership.objects.create(club=self.club, member=coach_member, season=self.season, status=ClubMembership.StatusChoices.ACTIVE) ClubMembership.objects.create(club=self.club, member=coach_member, season=self.season, status=ClubMembership.StatusChoices.ACTIVE)
self.client.force_login(coach) self.client.force_login(coach)
response = self.club_post("onboarding_requirement_create", {"name": "Photo", "order": "1"}) response = self.club_post("onboarding_requirement_create", {"name": "Photo"})
self.assertEqual(response.status_code, 403) self.assertEqual(response.status_code, 403)
self.assertFalse(OnboardingRequirement.objects.filter(club=self.club).exists()) self.assertFalse(OnboardingRequirement.objects.filter(club=self.club).exists())
@@ -6779,13 +6779,14 @@ class MemberRequirementChecklistTests(ManagementTestBase):
self.assertEqual(self.membership.status, ClubMembership.StatusChoices.ACTIVE) self.assertEqual(self.membership.status, ClubMembership.StatusChoices.ACTIVE)
self.assertEqual(self.membership.fee_status, ClubMembership.FeeStatus.PAID) self.assertEqual(self.membership.fee_status, ClubMembership.FeeStatus.PAID)
def test_a_coach_can_still_mark_one_complete(self): def test_a_plain_coach_cannot_mark_one_complete(self):
# Any staff can update a member's checklist -- same visibility as the rest of # Only an admin/MEMBER_ADMIN may touch a member's checklist
# their profile (ClubStaffRequiredMixin), not admin-only like defining the # (MemberAdminRequiredMixin) -- a coach can still *see* it on a member's
# requirement itself. That still means a *real* staff assignment though: a # profile (ClubStaffRequiredMixin's read access there), but not mark
# coach only sees members on a team they're staffed on (members_visible_to), # anything done or reopen it. A *real* staff assignment on the member's
# so the fixture needs to put self.member on that coach's own roster, not # own team is what would have made this coach able to reach the page at
# just any staff role in the club. # all pre-checklist-permission, so the fixture still sets that up to
# isolate this from a simpler "can't reach the page" 403.
team = Team.objects.create(club=self.club, name="U16") team = Team.objects.create(club=self.club, name="U16")
coach_position = Position.objects.create(club=self.club, name="Head Coach", staff_position=True, management_position=True) coach_position = Position.objects.create(club=self.club, name="Head Coach", staff_position=True, management_position=True)
player_position = Position.objects.create(club=self.club, name="Forward", staff_position=False) player_position = Position.objects.create(club=self.club, name="Forward", staff_position=False)
@@ -6800,7 +6801,25 @@ class MemberRequirementChecklistTests(ManagementTestBase):
response = self.club_post("member_requirement_complete", {"note": ""}, self.member.pk, self.requirement.pk) response = self.club_post("member_requirement_complete", {"note": ""}, self.member.pk, self.requirement.pk)
self.assertRedirects(response, reverse("management:member_detail", args=[self.member.pk])) self.assertEqual(response.status_code, 403)
def test_a_member_admin_can_mark_one_complete(self):
member_admin_user = User.objects.create_user(email="memberadmin-checklist@example.com", password="pw-secret-123")
member_admin_member = Member.objects.create(user=member_admin_user, first_name="Mia", last_name="Admin")
ClubMembership.objects.create(club=self.club, member=member_admin_member, season=self.season, status=ClubMembership.StatusChoices.ACTIVE)
# The active membership above already granted a plain MEMBER role via
# club/signals.py's post_save sync -- update that row rather than
# creating a second one (ClubRole is unique per club/member).
ClubRole.objects.filter(club=self.club, member=member_admin_member).update(role=ClubRole.Roles.MEMBER_ADMIN)
enrol_mfa(member_admin_user)
self.client.force_login(member_admin_user)
response = self.club_post("member_requirement_complete", {"note": ""}, self.member.pk, self.requirement.pk)
# fetch_redirect_response=False: this member_admin has no team/family tie
# to self.member, so members_visible_to would 404 the redirect target --
# a separate, pre-existing visibility question this test isn't about.
self.assertRedirects(response, reverse("management:member_detail", args=[self.member.pk]), fetch_redirect_response=False)
self.assertTrue(MemberRequirementStatus.objects.get(membership=self.membership, requirement=self.requirement).is_complete) self.assertTrue(MemberRequirementStatus.objects.get(membership=self.membership, requirement=self.requirement).is_complete)
def test_marking_incomplete_keeps_the_document_on_file(self): def test_marking_incomplete_keeps_the_document_on_file(self):

View File

@@ -3282,11 +3282,13 @@ def _current_membership_or_404(request, member_pk):
return membership return membership
class MemberRequirementCompleteView(ClubStaffRequiredMixin, View): class MemberRequirementCompleteView(MemberAdminRequiredMixin, View):
"""Mark one checklist item done for one membership -- any staff, not just """Mark one checklist item done for one membership -- admin/MEMBER_ADMIN
admins (same visibility as the rest of a member's profile), can record that only, same gate as the rest of people management (a plain coach can see a
a document came in. Reachable from the member detail page's Documents tab member's checklist on their profile, per ClubStaffRequiredMixin's read
(the default fallback) and from the admin-only Sign-up page (via `next`).""" access there, but not touch it). Reachable from the member detail page's
Documents tab (the default fallback) and from the admin-only Sign-up page
(via `next`)."""
def post(self, request, pk, requirement_pk): def post(self, request, pk, requirement_pk):
membership = _current_membership_or_404(request, pk) membership = _current_membership_or_404(request, pk)
@@ -3302,11 +3304,11 @@ class MemberRequirementCompleteView(ClubStaffRequiredMixin, View):
return _redirect_next_or(request, reverse("management:member_detail", args=[membership.member_id])) return _redirect_next_or(request, reverse("management:member_detail", args=[membership.member_id]))
class MemberRequirementBypassView(ClubStaffRequiredMixin, View): class MemberRequirementBypassView(MemberAdminRequiredMixin, View):
"""Confirm one checklist item isn't needed for this member (e.g. they already """Confirm one checklist item isn't needed for this member (e.g. they already
have a recent photo on file) -- see club.services.onboarding.mark_bypassed. have a recent photo on file) -- see club.services.onboarding.mark_bypassed.
Same visibility as MemberRequirementCompleteView; bypassing isn't a bigger Same gate as MemberRequirementCompleteView; bypassing isn't a bigger deal
deal than completing, it's just a different reason the item stops blocking than completing, it's just a different reason the item stops blocking
anything.""" anything."""
def post(self, request, pk, requirement_pk): def post(self, request, pk, requirement_pk):
@@ -3323,7 +3325,10 @@ class MemberRequirementBypassView(ClubStaffRequiredMixin, View):
return _redirect_next_or(request, reverse("management:member_detail", args=[membership.member_id])) return _redirect_next_or(request, reverse("management:member_detail", args=[membership.member_id]))
class MemberRequirementIncompleteView(ClubStaffRequiredMixin, View): class MemberRequirementIncompleteView(MemberAdminRequiredMixin, View):
"""Reopen a checklist item -- same admin/MEMBER_ADMIN gate as the other
two mutating requirement views above."""
def post(self, request, pk, requirement_pk): def post(self, request, pk, requirement_pk):
membership = _current_membership_or_404(request, pk) membership = _current_membership_or_404(request, pk)
requirement = get_object_or_404(OnboardingRequirement.objects.filter(club=request.club), pk=requirement_pk) requirement = get_object_or_404(OnboardingRequirement.objects.filter(club=request.club), pk=requirement_pk)

View File

@@ -2853,6 +2853,9 @@
} }
} }
} }
.isolate {
isolation: isolate;
}
.stack { .stack {
@layer daisyui.l1.l2.l3 { @layer daisyui.l1.l2.l3 {
display: inline-grid; display: inline-grid;