diff --git a/management/context_processors.py b/management/context_processors.py index b746a39..c49a5d8 100644 --- a/management/context_processors.py +++ b/management/context_processors.py @@ -36,6 +36,8 @@ _NAV_SECTIONS = { "membership_mark_fully_paid": "membership_list", "membership_record_payment": "membership_list", "position_list": "position_list", + "position_create": "position_list", + "position_update": "position_list", "role_list": "role_list", "role_create": "role_list", "role_revoke": "role_list", diff --git a/management/forms.py b/management/forms.py index 2596b72..fb781a1 100644 --- a/management/forms.py +++ b/management/forms.py @@ -7,7 +7,7 @@ from django.utils.translation import gettext_lazy as _ from club.models import ClubMembership, ClubRole, FeePayment from members.models import Family, FamilyMembership, Member from members.services.family import find_member_by_email -from teams.models import Team +from teams.models import Position, Team User = get_user_model() @@ -25,6 +25,20 @@ class TeamForm(forms.ModelForm): fields = ["name", "short_name"] +class PositionForm(forms.ModelForm): + class Meta: + model = Position + fields = ["name", "short_name", "ordering", "staff_position", "management_position"] + + def clean(self): + cleaned = super().clean() + # Mirrors Position's management_position_implies_staff_position check + # constraint -- caught here so it reads as a form error, not a 500. + if cleaned.get("management_position") and not cleaned.get("staff_position"): + self.add_error("management_position", _("A management position must also be a staff position.")) + return cleaned + + class ClubRoleAssignForm(forms.ModelForm): """Grant a club-wide role to a member already affiliated with this club.""" diff --git a/management/templates/management/position_form.html b/management/templates/management/position_form.html new file mode 100644 index 0000000..8e3670d --- /dev/null +++ b/management/templates/management/position_form.html @@ -0,0 +1,38 @@ +{% extends "management/base.html" %} +{% load i18n lucide ui %} + +{% block heading %}{% if update_view %}{% blocktrans %}Edit {{ object }}{% endblocktrans %}{% else %}{% trans "New position" %}{% endif %}{% endblock heading %} + +{% block panel %} +
+
+
+ {% csrf_token %} + + {% for error in form.non_field_errors %} +
+ {{ error }} +
+ {% endfor %} + +
+ {% form_field form.name %} + {% form_field form.short_name %} + {% form_field form.ordering %} +
+ +
+ +
+ {% form_field form.staff_position %} + {% form_field form.management_position %} +
+ +
+ {% lucide "arrow-left" size=16 %} {% trans "Cancel" %} + +
+
+
+
+{% endblock panel %} diff --git a/management/templates/management/position_list.html b/management/templates/management/position_list.html new file mode 100644 index 0000000..5b164f0 --- /dev/null +++ b/management/templates/management/position_list.html @@ -0,0 +1,55 @@ +{% extends "management/base.html" %} +{% load i18n lucide %} + +{% block heading %}{% trans "Positions" %}{% endblock heading %} + +{% block actions %} + {% lucide "plus" size=16 %} {% trans "New position" %} +{% endblock actions %} + +{% block panel %} +
+
+
+ + + + + + + + + + + + + {% for position in positions %} + + + + + + + + + {% empty %} + + + + {% endfor %} + +
{% trans "Ordering" %}{% trans "Name" %}{% trans "Short name" %}{% trans "Staff position" %}{% trans "Management position" %}
{{ position.ordering }}{{ position.name }}{{ position.short_name }} + + {% if position.staff_position %}{% trans "Yes" %}{% else %}{% trans "No" %}{% endif %} + + + + {% if position.management_position %}{% trans "Yes" %}{% else %}{% trans "No" %}{% endif %} + + + {% lucide "pencil" size=14 %} {% trans "Edit" %} +
{% trans "No positions yet." %}
+
+
+
+{% endblock panel %} diff --git a/management/templates/management/role_form.html b/management/templates/management/role_form.html deleted file mode 100644 index cfd5acc..0000000 --- a/management/templates/management/role_form.html +++ /dev/null @@ -1,29 +0,0 @@ -{% extends "management/base.html" %} -{% load i18n lucide ui %} - -{% block heading %}{% trans "Grant role" %}{% endblock heading %} - -{% block panel %} -
-
-
- {% csrf_token %} - - {% for error in form.non_field_errors %} -
- {{ error }} -
- {% endfor %} - - {% for field in form %} - {% form_field field %} - {% endfor %} - -
- {% lucide "arrow-left" size=16 %} {% trans "Cancel" %} - -
-
-
-
-{% endblock panel %} diff --git a/management/templates/management/role_list.html b/management/templates/management/role_list.html index ecccd27..3a35795 100644 --- a/management/templates/management/role_list.html +++ b/management/templates/management/role_list.html @@ -1,44 +1,154 @@ {% extends "management/base.html" %} {% load i18n lucide %} +{% comment %} + One section per non-MEMBER role (management.views.ClubRoleListView) -- the + plain MEMBER role every active club member holds automatically is excluded + entirely, since listing it here would just be noise. "Grant role" opens a + modal (controlpanel/_modal_form.html) rather than a separate page -- role_create + is POST-only, see ClubRoleCreateView. +{% endcomment %} + {% block heading %}{% trans "Roles" %}{% endblock heading %} {% block actions %} - {% lucide "plus" size=16 %} {% trans "Grant role" %} + {% endblock actions %} {% block panel %} -
-
-
- - - - - - - - - - {% for role in roles %} - - - - - - {% empty %} - - - - {% endfor %} - -
{% trans "Member" %}{% trans "Role" %}
{{ role.member }}{{ role.get_role_display }} -
- {% csrf_token %} - -
-
{% trans "No roles granted yet." %}
-
-
+
+ {% for value, label, description, roles_for_section in sections %} + {% with role_label=label|capfirst %} +
+
+

{{ role_label }}

+ {% if description %}

{{ description }}

{% endif %} +
+ + + + + + + + + {% for role in roles_for_section %} + + + + + {% empty %} + + + + {% endfor %} + +
{% trans "Member" %}
{{ role.member }} +
+ {% csrf_token %} + +
+
{% blocktrans %}No one has the {{ role_label }} role yet.{% endblocktrans %}
+
+
+
+ {% endwith %} + {% endfor %}
+ + {% trans "Grant role" as grant_role_label %} + {% url 'management:role_create' as role_create_url %} + {% include "controlpanel/_modal_form.html" with modal_id="grant_role_modal" title=grant_role_label form=role_form action_url=role_create_url submit_label=grant_role_label submit_icon="shield-check" %} {% endblock panel %} + +{% block extra_body %} + {% trans "Type a name to search..." as search_placeholder %} + +{% endblock extra_body %} diff --git a/management/tests.py b/management/tests.py index 446fb3f..ac9880a 100644 --- a/management/tests.py +++ b/management/tests.py @@ -256,6 +256,43 @@ class TeamManagementTests(ManagementTestBase): self.assertRedirects(response, reverse("management:team_detail", args=[team.pk])) +class PositionManagementTests(ManagementTestBase): + def setUp(self): + super().setUp() + self.client.force_login(self.admin_user) + + def test_position_list_is_scoped_to_the_club(self): + other_club = Club.objects.create(name="Rival FC", slug="rival-fc") + Position.objects.create(club=other_club, name="Rival Coach", short_name="RC", staff_position=True) + + response = self.club_get("position_list") + + self.assertNotContains(response, "Rival Coach") + + def test_creating_a_position(self): + response = self.club_post("position_create", {"name": "Physio", "short_name": "PH", "ordering": 0, "staff_position": "on", "management_position": ""}) + + position = Position.objects.get(club=self.club, name="Physio") + self.assertRedirects(response, reverse("management:position_list")) + self.assertTrue(position.staff_position) + self.assertFalse(position.management_position) + + def test_updating_a_position(self): + position = Position.objects.create(club=self.club, name="Old name", short_name="ON") + + self.club_post("position_update", {"name": "New name", "short_name": "NN", "ordering": 0}, position.pk) + + position.refresh_from_db() + self.assertEqual(position.name, "New name") + + def test_a_management_position_must_also_be_a_staff_position(self): + response = self.club_post("position_create", {"name": "Bad", "short_name": "B", "ordering": 0, "management_position": "on"}) + + self.assertEqual(response.status_code, 200) + self.assertFalse(Position.objects.filter(club=self.club, name="Bad").exists()) + self.assertFormError(response.context["form"], "management_position", "A management position must also be a staff position.") + + class ClubRoleManagementTests(ManagementTestBase): def setUp(self): super().setUp() @@ -279,6 +316,37 @@ class ClubRoleManagementTests(ManagementTestBase): self.assertFalse(ClubRole.objects.filter(pk=role.pk).exists()) + def test_the_plain_member_role_never_appears_on_the_list(self): + # self.admin_member and self.member both hold an implicit MEMBER role -- + # noise this page must never show. (self.member's name still legitimately + # appears once, in the "Grant role" modal's member picker.) + response = self.club_get("role_list") + + self.assertContains(response, "No one has the Editor role yet.") + + def test_a_granted_role_appears_under_its_own_section(self): + self.club_post("role_create", {"member": str(self.member.pk), "role": ClubRole.Roles.EDITOR}) + + response = self.club_get("role_list") + + self.assertContains(response, "Future Editor") + + def test_grant_role_is_a_modal_on_the_list_page(self): + response = self.club_get("role_list") + + self.assertContains(response, 'id="grant_role_modal"') + + def test_each_section_explains_what_the_role_grants(self): + response = self.club_get("role_list") + + self.assertContains(response, "Full control over the club") + self.assertContains(response, "Can create and edit events") + + def test_an_invalid_submission_redirects_back_to_the_list_instead_of_a_page(self): + response = self.club_post("role_create", {"member": "", "role": ClubRole.Roles.EDITOR}) + + self.assertRedirects(response, reverse("management:role_list")) + class FamilyManagementTests(ManagementTestBase): def setUp(self): diff --git a/management/urls.py b/management/urls.py index b567e9f..890f053 100644 --- a/management/urls.py +++ b/management/urls.py @@ -30,6 +30,8 @@ urlpatterns = [ path("families//members//role/", views.FamilyMembershipRoleUpdateView.as_view(), name="family_membership_role_update"), # Club setup (admin only) path("positions/", views.PositionListView.as_view(), name="position_list"), + path("positions/new/", views.PositionCreateView.as_view(), name="position_create"), + path("positions//edit/", views.PositionUpdateView.as_view(), name="position_update"), path("roles/", views.ClubRoleListView.as_view(), name="role_list"), path("roles/new/", views.ClubRoleCreateView.as_view(), name="role_create"), path("roles//revoke/", views.ClubRoleRevokeView.as_view(), name="role_revoke"), diff --git a/management/views.py b/management/views.py index 8422dea..2661576 100644 --- a/management/views.py +++ b/management/views.py @@ -24,7 +24,20 @@ from shop.models import Discount, Invoice, Order, Product from teams.models import Position, StaffAssignment, Team, TeamMembership from .bulk_import import build_member_import_template, parse_member_import_rows, read_member_import_workbook -from .forms import AddChildForm, AddParentForm, AttachToFamilyForm, ClubMembershipForm, ClubRoleAssignForm, FamilyCreateForm, GrantLoginForm, MemberForm, MemberImportUploadForm, RecordFeePaymentForm, TeamForm +from .forms import ( + AddChildForm, + AddParentForm, + AttachToFamilyForm, + ClubMembershipForm, + ClubRoleAssignForm, + FamilyCreateForm, + GrantLoginForm, + MemberForm, + MemberImportUploadForm, + PositionForm, + RecordFeePaymentForm, + TeamForm, +) from .pdf import PDFExportError, membership_list_pdf @@ -707,18 +720,38 @@ class TeamDetailView(ClubStaffRequiredMixin, DetailView): # granted or taken away) ------------------------------------------------------------- +#: What each non-default role actually grants -- shown on the roles overview so an +#: admin granting one knows what they're handing out. See club/services/access.py. +ROLE_DESCRIPTIONS = { + ClubRole.Roles.ADMIN: _("Full control over the club: memberships, positions, roles, teams, shop, and every event."), + ClubRole.Roles.EDITOR: _("Can create and edit events, but not memberships, positions, roles, or shop settings."), +} + + class ClubRoleListView(ClubAdminRequiredMixin, ListView): + """Every non-default role holder, grouped by role. The plain MEMBER role is + excluded entirely -- every active club member holds it automatically + (club/signals.py), so listing it here would just be noise; this page is for + the roles someone was actually *granted*. ClubRole.Roles has few enough + values that a section per role reads better than one flat table.""" + template_name = "management/role_list.html" context_object_name = "roles" def get_queryset(self): - return ClubRole.objects.filter(club=self.request.club).select_related("member") + return ClubRole.objects.filter(club=self.request.club).exclude(role=ClubRole.Roles.MEMBER).select_related("member") + + def get_context_data(self, **kwargs): + sections = [(value, label, ROLE_DESCRIPTIONS.get(value, ""), [role for role in self.object_list if role.role == value]) for value, label in ClubRole.Roles.choices if value != ClubRole.Roles.MEMBER] + return super().get_context_data(sections=sections, role_form=ClubRoleAssignForm(club=self.request.club), **kwargs) -class ClubRoleCreateView(ClubAdminRequiredMixin, CreateView): - model = ClubRole +class ClubRoleCreateView(ClubAdminRequiredMixin, RedirectOnInvalidMixin, FormView): + """Reachable only via the "Grant role" modal on the roles overview.""" + form_class = ClubRoleAssignForm - template_name = "management/role_form.html" + http_method_names = ["post"] + invalid_redirect_url_name = "management:role_list" def get_form_kwargs(self): return super().get_form_kwargs() | {"club": self.request.club} @@ -729,17 +762,14 @@ class ClubRoleCreateView(ClubAdminRequiredMixin, CreateView): # so granting ADMIN/EDITOR promotes that existing row rather than inserting a # second one, exactly like controlpanel.services.admins.grant_club_admin. member, role = form.cleaned_data["member"], form.cleaned_data["role"] - self.object, created = ClubRole.objects.get_or_create(club=self.request.club, member=member, defaults={"role": role}) - if not created and self.object.role != role: - self.object.role = role - self.object.save(update_fields=["role"]) + role_obj, created = ClubRole.objects.get_or_create(club=self.request.club, member=member, defaults={"role": role}) + if not created and role_obj.role != role: + role_obj.role = role + role_obj.save(update_fields=["role"]) - body = _("“%(member)s” is now %(role)s.") % {"member": member, "role": self.object.get_role_display()} + body = _("“%(member)s” is now %(role)s.") % {"member": member, "role": role_obj.get_role_display()} notify(self.request, f"s|{_('Role granted')}|{body}") - return redirect(self.get_success_url()) - - def get_success_url(self): - return reverse("management:role_list") + return redirect("management:role_list") class ClubRoleRevokeView(ClubAdminRequiredMixin, View): @@ -875,13 +905,51 @@ class FamilyAddParentView(ClubAdminRequiredMixin, RedirectOnInvalidMixin, FormVi return redirect("management:family_detail", pk=family.pk) -class PositionListView(ClubAdminRequiredMixin, StubListMixin, ListView): - page_title = _("Positions") +class PositionListView(ClubAdminRequiredMixin, ListView): + template_name = "management/position_list.html" + context_object_name = "positions" def get_queryset(self): return Position.objects.filter(club=self.request.club) +class PositionCreateView(ClubAdminRequiredMixin, CreateView): + model = Position + form_class = PositionForm + template_name = "management/position_form.html" + + def form_valid(self, form): + form.instance.club = self.request.club + response = super().form_valid(form) + body = _("“%(position)s” created.") % {"position": self.object} + notify(self.request, f"s|{_('Position created')}|{body}") + return response + + def get_success_url(self): + return reverse("management:position_list") + + +class PositionUpdateView(ClubAdminRequiredMixin, UpdateView): + model = Position + form_class = PositionForm + template_name = "management/position_form.html" + + def get_queryset(self): + return Position.objects.filter(club=self.request.club) + + def form_valid(self, form): + response = super().form_valid(form) + body = _("“%(position)s” updated.") % {"position": self.object} + notify(self.request, f"s|{_('Position updated')}|{body}") + return response + + def get_success_url(self): + return reverse("management:position_list") + + def get_context_data(self, **kwargs): + return super().get_context_data(update_view=True, **kwargs) + + class RosterListView(ClubStaffRequiredMixin, StubListMixin, ListView): page_title = _("Roster") diff --git a/static/css/app.css b/static/css/app.css index 4191754..65efb5b 100644 --- a/static/css/app.css +++ b/static/css/app.css @@ -2213,9 +2213,15 @@ } } } + .absolute { + position: absolute; + } .fixed { position: fixed; } + .relative { + position: relative; + } .static { position: static; } @@ -2250,6 +2256,12 @@ } } } + .top-1 { + top: var(--spacing); + } + .left-1 { + left: var(--spacing); + } .file-input { @layer daisyui.l1.l2.l3 { cursor: pointer; @@ -2620,6 +2632,9 @@ .z-10 { z-index: 10; } + .col-span-full { + grid-column: 1 / -1; + } .hero { @layer daisyui.l1.l2.l3 { display: grid; @@ -3060,6 +3075,9 @@ .table { display: table; } + .aspect-square { + aspect-ratio: 1 / 1; + } .h-12 { height: calc(var(--spacing) * 12); } @@ -3075,6 +3093,9 @@ .h-screen { height: 100vh; } + .max-h-60 { + max-height: calc(var(--spacing) * 60); + } .min-h-6 { min-height: calc(var(--spacing) * 6); } @@ -3156,12 +3177,18 @@ .grid-cols-1 { grid-template-columns: repeat(1, minmax(0, 1fr)); } + .grid-cols-2 { + grid-template-columns: repeat(2, minmax(0, 1fr)); + } .flex-col { flex-direction: column; } .flex-row { flex-direction: row; } + .flex-nowrap { + flex-wrap: nowrap; + } .flex-wrap { flex-wrap: wrap; } @@ -3434,6 +3461,9 @@ .object-contain { object-fit: contain; } + .object-cover { + object-fit: cover; + } .p-2 { padding: calc(var(--spacing) * 2); } @@ -3564,6 +3594,9 @@ .whitespace-nowrap { white-space: nowrap; } + .whitespace-pre-line { + white-space: pre-line; + } .alert-error { @layer daisyui.l1.l2 { color: var(--color-error-content); @@ -3717,6 +3750,10 @@ outline-style: var(--tw-outline-style); outline-width: 1px; } + .blur { + --tw-blur: blur(8px); + filter: var(--tw-blur,) var(--tw-brightness,) var(--tw-contrast,) var(--tw-grayscale,) var(--tw-hue-rotate,) var(--tw-invert,) var(--tw-saturate,) var(--tw-sepia,) var(--tw-drop-shadow,); + } .filter { filter: var(--tw-blur,) var(--tw-brightness,) var(--tw-contrast,) var(--tw-grayscale,) var(--tw-hue-rotate,) var(--tw-invert,) var(--tw-saturate,) var(--tw-sepia,) var(--tw-drop-shadow,); } @@ -3799,6 +3836,13 @@ --btn-soft-bg: initial; } } + .btn-xs { + @layer daisyui.l1.l2 { + --fontsize: 0.6875rem; + --btn-p: 0.5rem; + --size: calc(var(--size-field, 0.25rem) * 6); + } + } .badge-accent { @layer daisyui.l1.l2 { --badge-color: var(--color-accent); @@ -3881,6 +3925,11 @@ grid-template-columns: repeat(2, minmax(0, 1fr)); } } + .md\:grid-cols-3 { + @media (width >= 48rem) { + grid-template-columns: repeat(3, minmax(0, 1fr)); + } + } .md\:grid-cols-4 { @media (width >= 48rem) { grid-template-columns: repeat(4, minmax(0, 1fr)); diff --git a/teams/migrations/0005_alter_position_management_position_and_more.py b/teams/migrations/0005_alter_position_management_position_and_more.py new file mode 100644 index 0000000..f5e5f5a --- /dev/null +++ b/teams/migrations/0005_alter_position_management_position_and_more.py @@ -0,0 +1,23 @@ +# Generated by Django 6.0.6 on 2026-08-03 15:55 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('teams', '0004_position_created_position_modified_and_more'), + ] + + operations = [ + migrations.AlterField( + model_name='position', + name='management_position', + field=models.BooleanField(default=False, help_text='A staff position with management authority over the team (e.g. head coach) -- requires staff position to also be checked.', verbose_name='management position'), + ), + migrations.AlterField( + model_name='position', + name='staff_position', + field=models.BooleanField(default=False, help_text='Coach, manager, physio, ... -- assignable via a StaffAssignment rather than a team roster spot.', verbose_name='staff position'), + ), + ] diff --git a/teams/migrations/0006_alter_position_ordering.py b/teams/migrations/0006_alter_position_ordering.py new file mode 100644 index 0000000..3752819 --- /dev/null +++ b/teams/migrations/0006_alter_position_ordering.py @@ -0,0 +1,18 @@ +# Generated by Django 6.0.6 on 2026-08-03 15:58 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('teams', '0005_alter_position_management_position_and_more'), + ] + + operations = [ + migrations.AlterField( + model_name='position', + name='ordering', + field=models.PositiveSmallIntegerField(default=0, help_text="Lower numbers are listed first (e.g. on a team's roster). Positions with the same number are ordered by name.", verbose_name='ordering'), + ), + ] diff --git a/teams/models.py b/teams/models.py index f643ef0..bdeb154 100644 --- a/teams/models.py +++ b/teams/models.py @@ -25,10 +25,10 @@ class Team(ClubScopedModel): class Position(ClubScopedModel): name = models.CharField(_("name"), max_length=255) short_name = models.CharField(_("short name"), max_length=255) - ordering = models.PositiveSmallIntegerField(_("ordering"), default=0) + ordering = models.PositiveSmallIntegerField(_("ordering"), default=0, help_text=_("Lower numbers are listed first (e.g. on a team's roster). Positions with the same number are ordered by name.")) - staff_position = models.BooleanField(_("staff position"), default=False) - management_position = models.BooleanField(_("management position"), default=False) + staff_position = models.BooleanField(_("staff position"), default=False, help_text=_("Coach, manager, physio, ... -- assignable via a StaffAssignment rather than a team roster spot.")) + management_position = models.BooleanField(_("management position"), default=False, help_text=_("A staff position with management authority over the team (e.g. head coach) -- requires staff position to also be checked.")) class Meta: verbose_name = _("position")