diff --git a/club/admin.py b/club/admin.py index 24fb5c6..16926fc 100644 --- a/club/admin.py +++ b/club/admin.py @@ -67,10 +67,10 @@ class ClubRoleAdmin(admin.ModelAdmin): @admin.register(OnboardingRequirement) 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"] search_fields = ["name", "club__name"] - ordering = ["club", "order", "name"] + ordering = ["club", "name"] @admin.register(MemberRequirementStatus) diff --git a/club/migrations/0028_alter_onboardingrequirement_options_and_more.py b/club/migrations/0028_alter_onboardingrequirement_options_and_more.py new file mode 100644 index 0000000..ff51c8b --- /dev/null +++ b/club/migrations/0028_alter_onboardingrequirement_options_and_more.py @@ -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', + ), + ] diff --git a/club/models.py b/club/models.py index 3ed2e0c..c6894ca 100644 --- a/club/models.py +++ b/club/models.py @@ -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.")) 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.")) - order = models.PositiveIntegerField(_("order"), default=0, help_text=_("Lower numbers show first on the checklist.")) class Meta: verbose_name = _("onboarding requirement") 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 = [ models.UniqueConstraint(fields=["club", "name"], name="unique_onboarding_requirement_name_per_club"), ] diff --git a/club/tests.py b/club/tests.py index a72b94b..86891aa 100644 --- a/club/tests.py +++ b/club/tests.py @@ -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 ) 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.medical = OnboardingRequirement.objects.create(club=cls.club, name="Medical certificate", requires_document=True, order=2) + cls.photo = OnboardingRequirement.objects.create(club=cls.club, name="Photo") + 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): self.assertEqual(self.membership.open_requirement_count, 2) diff --git a/management/forms.py b/management/forms.py index 5f81d8f..59e1808 100644 --- a/management/forms.py +++ b/management/forms.py @@ -904,7 +904,7 @@ class OnboardingRequirementForm(forms.ModelForm): class Meta: 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): diff --git a/management/templates/management/member_detail.html b/management/templates/management/member_detail.html index 4b24ba5..ecaa1cd 100644 --- a/management/templates/management/member_detail.html +++ b/management/templates/management/member_detail.html @@ -203,10 +203,12 @@ {% comment %} Onboarding checklist -- club.services.onboarding.checklist_for, paired with - MemberRequirementStatus if one exists. Any staff can mark an item done or - reopen it, not just admins (same visibility as the rest of this page) -- - see the module docstring on club/services/onboarding.py for why this stays - separate from ClubMembership.status/fee_status. + MemberRequirementStatus if one exists. Any staff can see it (same + visibility as the rest of this page), but only an admin/MEMBER_ADMIN can + mark an item done or reopen it (MemberAdminRequiredMixin on the three + 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 %}

{% lucide "clipboard-check" size=18 %} {% trans "Documents" %}

@@ -238,16 +240,18 @@ {% lucide "download" size=13 %} {% trans "Download document" %} {% endif %}
-
- {% if status.is_complete %} -
- {% csrf_token %} - -
- {% else %} - - {% endif %} -
+ {% if can_manage_members %} +
+ {% if status.is_complete %} +
+ {% csrf_token %} + +
+ {% else %} + + {% endif %} +
+ {% endif %} {% endfor %} @@ -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 %} {% endif %} + {% if can_manage_members %} {% for requirement, status in checklist %} {% if not status.is_complete %} @@ -395,6 +400,7 @@ {% endif %} {% endfor %} + {% endif %} {% endblock panel %} {% block extra_body %} diff --git a/management/templates/management/onboarding_requirement_list.html b/management/templates/management/onboarding_requirement_list.html index 65d0d55..0ff8e72 100644 --- a/management/templates/management/onboarding_requirement_list.html +++ b/management/templates/management/onboarding_requirement_list.html @@ -3,11 +3,11 @@ {% comment %} What this club requires from every member after they sign up or renew -- - see club/models.py's OnboardingRequirement docstring. Order matters (shown - as typed in `order`, ascending) -- it's the sequence staff see on a - member's checklist. No dedicated mockup screen for this (new feature, not - in the original design file) -- extrapolates the card/table vocabulary - used across Settings. + see club/models.py's OnboardingRequirement docstring. Listed alphabetically, + not in a configurable sequence: every active requirement blocks equally and + there's no set order to complete them in. No dedicated mockup screen for + this (new feature, not in the original design file) -- extrapolates the + card/table vocabulary used across Settings. {% endcomment %} {% block panel_title %}{% trans "Onboarding requirements" %}{% endblock panel_title %} diff --git a/management/tests.py b/management/tests.py index a5f2da3..920092b 100644 --- a/management/tests.py +++ b/management/tests.py @@ -6702,7 +6702,7 @@ class OnboardingRequirementManagementTests(ManagementTestBase): self.client.force_login(self.admin_user) 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.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) 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.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.fee_status, ClubMembership.FeeStatus.PAID) - def test_a_coach_can_still_mark_one_complete(self): - # Any staff can update a member's checklist -- same visibility as the rest of - # their profile (ClubStaffRequiredMixin), not admin-only like defining the - # requirement itself. That still means a *real* staff assignment though: a - # coach only sees members on a team they're staffed on (members_visible_to), - # so the fixture needs to put self.member on that coach's own roster, not - # just any staff role in the club. + def test_a_plain_coach_cannot_mark_one_complete(self): + # Only an admin/MEMBER_ADMIN may touch a member's checklist + # (MemberAdminRequiredMixin) -- a coach can still *see* it on a member's + # profile (ClubStaffRequiredMixin's read access there), but not mark + # anything done or reopen it. A *real* staff assignment on the member's + # own team is what would have made this coach able to reach the page at + # 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") 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) @@ -6800,7 +6801,25 @@ class MemberRequirementChecklistTests(ManagementTestBase): 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) def test_marking_incomplete_keeps_the_document_on_file(self): diff --git a/management/views.py b/management/views.py index 4ed148c..aa6c95b 100644 --- a/management/views.py +++ b/management/views.py @@ -3282,11 +3282,13 @@ def _current_membership_or_404(request, member_pk): return membership -class MemberRequirementCompleteView(ClubStaffRequiredMixin, View): - """Mark one checklist item done for one membership -- any staff, not just - admins (same visibility as the rest of a member's profile), can record that - a document came in. Reachable from the member detail page's Documents tab - (the default fallback) and from the admin-only Sign-up page (via `next`).""" +class MemberRequirementCompleteView(MemberAdminRequiredMixin, View): + """Mark one checklist item done for one membership -- admin/MEMBER_ADMIN + only, same gate as the rest of people management (a plain coach can see a + member's checklist on their profile, per ClubStaffRequiredMixin's read + 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): 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])) -class MemberRequirementBypassView(ClubStaffRequiredMixin, View): +class MemberRequirementBypassView(MemberAdminRequiredMixin, View): """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. - Same visibility as MemberRequirementCompleteView; bypassing isn't a bigger - deal than completing, it's just a different reason the item stops blocking + Same gate as MemberRequirementCompleteView; bypassing isn't a bigger deal + than completing, it's just a different reason the item stops blocking anything.""" 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])) -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): membership = _current_membership_or_404(request, pk) requirement = get_object_or_404(OnboardingRequirement.objects.filter(club=request.club), pk=requirement_pk) diff --git a/static/css/app.css b/static/css/app.css index ff2455f..e8ae14c 100644 --- a/static/css/app.css +++ b/static/css/app.css @@ -2853,6 +2853,9 @@ } } } + .isolate { + isolation: isolate; + } .stack { @layer daisyui.l1.l2.l3 { display: inline-grid;