bernard/RosterChief
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Compare commits

base: bernard:127d0e338e1b7f755ca31e457b0a1b652613af7a
Branches Tags
bernard:main bernard:development
bernard:v0.1.0
...
compare: bernard:development
Branches Tags
bernard:main bernard:development
bernard:v0.1.0

1 Commits
127d0e338e ... developmen

Author SHA1 Message Date
Bernard Siebens
40255805c3 Modularize confirmation modal for destructive POST actions and add notify helper for concise message handling across the UI. 2026-07-16 18:34:03 +02:00
19 changed files with 369 additions and 221 deletions
Expand all files Collapse all files

35
controlpanel/messages.py Normal file
View File

@@ -0,0 +1,35 @@
"""A compact way to queue a Django message that carries its own title.
Django's messages framework has no title field — a call site that wants one passes it
as ``extra_tags`` (``messages.success(request, body, extra_tags="Club created")``), which
reads fine written out but is easy to forget, so in practice every message ends up on
the generic per-level heading (`as_alert`'s "Done" / "Careful" / "Something went wrong").
``notify`` folds level, title and body into one string instead: ``"<level>|<title>|<body>"``.
One call, title included, nothing to forget. `as_alert` (controlpanel/templatetags/ui.py)
reads the title back off ``extra_tags`` at render time — unchanged from before.
"""
from django.contrib import messages
#: One letter per Django message level. `notify` picks the level from the spec string;
#: `as_alert` picks the icon/colour/fallback-title from the level the message actually
#: carries (via ``level_tag``), so the two stay in step by construction.
LEVELS = {
"s": messages.SUCCESS,
"i": messages.INFO,
"w": messages.WARNING,
"e": messages.ERROR,
"d": messages.DEBUG,
}
def notify(request, spec: str, **kwargs) -> None:
"""Queue a message from a ``"<level>|<title>|<body>"`` spec.
``level`` is one of ``s`` (success), ``i`` (info), ``w`` (warning), ``e`` (error),
``d`` (debug). An empty title (``"s||Body text"``) falls back to the generic
per-level heading, same as never passing ``extra_tags`` at all.
"""
level_code, title, body = spec.split("|", 2)
messages.add_message(request, LEVELS[level_code], body, extra_tags=title, **kwargs)

5
controlpanel/mixins.py
View File

@@ -1,8 +1,9 @@
from django.contrib import messages
from django.contrib.auth.mixins import UserPassesTestMixin
from django.http import Http404
from django.shortcuts import redirect
from .messages import notify
class PlatformStaffRequiredMixin(UserPassesTestMixin):
"""Gate for the platform control panel.
@@ -56,5 +57,5 @@ class RedirectOnInvalidMixin:
def form_invalid(self, form):
for error in form.errors.values():
messages.error(self.request, " ".join(error))
notify(self.request, f"e|Couldn't save|{' '.join(error)}")
return redirect(self.invalid_redirect_url_name, **self.get_invalid_redirect_kwargs())

22
controlpanel/templates/controlpanel/_club_admins_card.html
View File

@@ -1,14 +1,14 @@
{% load lucide %}
{% load lucide ui %}
{% comment %}
Club-scoped admins, and the form to add one. Included with `club`, `admins` already
in context.
Club-scoped admins, and the modals to add one / confirm removing one. Included with
`club`, `admins`, `admin_form` already in context.
{% endcomment %}
<div class="card bg-base-100 shadow">
<div class="card-body">
<div class="flex items-center justify-between">
<h2 class="card-title text-base">{% lucide "shield-user" size=18 %} Club admins</h2>
<a class="btn btn-primary btn-sm gap-2" href="{% url 'controlpanel:club_admin_add' club.pk %}">{% lucide "user-plus" size=16 %} Add admin</a>
<button class="btn btn-primary btn-sm gap-2" type="button" onclick="document.getElementById('club_admin_add_modal').showModal()">{% lucide "user-plus" size=16 %} Add admin</button>
</div>
<div class="overflow-x-auto">
<table class="table">
@@ -25,10 +25,7 @@
<td>{{ role.member }}</td>
<td>{{ role.member.user.email|default:"—" }}</td>
<td class="text-right">
<form method="post" action="{% url 'controlpanel:club_admin_remove' club.pk role.pk %}">
{% csrf_token %}
<button class="btn btn-error btn-outline btn-sm gap-1" type="submit">{% lucide "trash-2" size=14 %} Remove</button>
</form>
<button class="btn btn-error btn-outline btn-sm gap-1" type="button" onclick="document.getElementById('{{ role.pk|dom_id:"admin_remove_modal" }}').showModal()">{% lucide "trash-2" size=14 %} Remove</button>
</td>
</tr>
{% empty %}
@@ -41,3 +38,12 @@
</div>
</div>
</div>
{% url 'controlpanel:club_admin_add' club.pk as club_admin_add_url %}
{% include "controlpanel/_modal_form.html" with modal_id="club_admin_add_modal" title="Add admin" form=admin_form action_url=club_admin_add_url submit_label="Grant admin" submit_icon="user-plus" blurb="A club admin can manage everything in this club. They will be required to set up two-factor authentication before they can sign in." %}
{% comment %} Dialogs live outside the table: <tbody> may only contain <tr> elements. {% endcomment %}
{% for role in admins %}
{% url 'controlpanel:club_admin_remove' club.pk role.pk as admin_remove_url %}
{% include "controlpanel/_confirm_modal.html" with modal_id=role.pk|dom_id:"admin_remove_modal" title="Remove admin" body="Remove "|add:role.member.get_full_name|add:" as an admin of this club? They keep their membership — only admin rights are revoked." action_url=admin_remove_url submit_label="Remove" %}
{% endfor %}

28
controlpanel/templates/controlpanel/_confirm_modal.html Normal file
View File

@@ -0,0 +1,28 @@
{% load lucide %}
{% comment %}
A daisyUI native <dialog> confirmation modal for a destructive POST action with no
fields of its own. Included with `modal_id`, `title`, `body`, `action_url`, and
optional `submit_label` (default "Confirm"), `submit_icon` (default "trash-2"). The
submit button sits outside the form tag (linked via the `form` attribute), same as
`_modal_form.html`, so it can share the `modal-action` row with the dialog-closing
Cancel button without nesting one <form> inside another.
{% endcomment %}
<dialog id="{{ modal_id }}" class="modal">
<div class="modal-box">
<h3 class="text-lg font-bold">{{ title }}</h3>
<p class="py-2 text-sm opacity-70">{{ body }}</p>
<form method="post" action="{{ action_url }}" id="{{ modal_id }}-form">
{% csrf_token %}
</form>
<div class="modal-action">
<form method="dialog">
<button class="btn btn-outline gap-2">{% lucide "x" size=16 %} Cancel</button>
</form>
<button class="btn btn-error gap-2" type="submit" form="{{ modal_id }}-form">{% lucide submit_icon|default:"trash-2" size=16 %} {{ submit_label|default:"Confirm" }}</button>
</div>
</div>
<form method="dialog" class="modal-backdrop">
<button>close</button>
</form>
</dialog>

2
controlpanel/templates/controlpanel/_nav_items.html
View File

@@ -31,7 +31,7 @@
{# Superusers only, exactly as the view is gated: a link staff cannot follow is a lie. #}
<li>
<a class="{% if nav == 'admins' %}menu-active{% endif %}" href="{% url 'controlpanel:admins' %}">
{% lucide "user-cog" size=16 %} Admins
{% lucide "user-cog" size=16 %} Platform admins
</a>
</li>
{% endif %}

22
controlpanel/templates/controlpanel/admin_form.html
View File

@@ -1,22 +0,0 @@
{% extends "controlpanel/base.html" %}
{% load lucide %}
{% block heading %}Grant platform access{% endblock heading %}
{% block panel %}
<div class="card max-w-xl bg-base-100 shadow">
<div class="card-body">
<div class="alert alert-info">
<span>Platform admins can manage every club. They must set up two-factor authentication before they can sign in.</span>
</div>
<form method="post">
{% csrf_token %}
{% include "controlpanel/_form_fields.html" %}
<div class="card-actions justify-end pt-2">
<a class="btn btn-outline gap-2" href="{% url 'controlpanel:admins' %}">{% lucide "arrow-left" size=16 %} Cancel</a>
<button class="btn btn-primary gap-2" type="submit">{% lucide "user-plus" size=16 %} Grant access</button>
</div>
</form>
</div>
</div>
{% endblock panel %}

29
controlpanel/templates/controlpanel/admins.html
View File

@@ -1,5 +1,5 @@
{% extends "controlpanel/base.html" %}
{% load lucide %}
{% load lucide ui %}
{% block heading %}Platform admins{% endblock heading %}
@@ -8,10 +8,13 @@
{% endblock subheading %}
{% block actions %}
<a class="btn btn-primary gap-2" href="{% url 'controlpanel:admin_add' %}">{% lucide "user-plus" size=16 %} Grant access</a>
<button class="btn btn-primary gap-2" type="button" onclick="document.getElementById('admin_add_modal').showModal()">{% lucide "user-plus" size=16 %} Grant access</button>
{% endblock actions %}
{% block panel %}
{% url 'controlpanel:admin_add' as admin_add_url %}
{% include "controlpanel/_modal_form.html" with modal_id="admin_add_modal" title="Grant platform access" form=admin_form action_url=admin_add_url submit_label="Grant access" submit_icon="user-plus" blurb="Platform admins can manage every club. They must set up two-factor authentication before they can sign in." %}
<div class="card bg-base-100 shadow">
<div class="card-body">
<div class="overflow-x-auto">
@@ -30,15 +33,16 @@
<tr>
<td>
<div class="font-medium">{{ admin.email }}</div>
{% if admin.pk == user.pk %}<div class="text-xs opacity-60">That's you</div>{% endif %}
{% if admin.pk == user.pk %}
<div class="text-xs opacity-60">That's you</div>{% endif %}
</td>
<td>
<form method="post" action="{% url 'controlpanel:admin_update' admin.pk %}">
{% csrf_token %}
<input type="hidden" name="is_staff" value="{% if admin.is_staff %}0{% else %}1{% endif %}">
<input type="hidden" name="is_superuser" value="{% if admin.is_superuser %}1{% else %}0{% endif %}">
<button class="btn btn-xs gap-1 {% if admin.is_staff %}btn-success{% else %}btn-ghost{% endif %}" type="submit">
{% if admin.is_staff %}{% lucide "check" size=14 %} Yes{% else %}No{% endif %}
<button class="btn btn-sm gap-1 {% if admin.is_staff %}btn-success{% else %}btn-outline{% endif %}" type="submit">
{% if admin.is_staff %}{% lucide "user" size=14 %} Yes{% else %}{% lucide "x" size=14 %} No{% endif %}
</button>
</form>
</td>
@@ -47,17 +51,14 @@
{% csrf_token %}
<input type="hidden" name="is_staff" value="{% if admin.is_staff %}1{% else %}0{% endif %}">
<input type="hidden" name="is_superuser" value="{% if admin.is_superuser %}0{% else %}1{% endif %}">
<button class="btn btn-xs gap-1 {% if admin.is_superuser %}btn-warning{% else %}btn-ghost{% endif %}" type="submit">
{% if admin.is_superuser %}{% lucide "shield" size=14 %} Yes{% else %}No{% endif %}
<button class="btn btn-sm gap-1 {% if admin.is_superuser %}btn-warning{% else %}btn-outline{% endif %}" type="submit">
{% if admin.is_superuser %}{% lucide "shield" size=14 %} Yes{% else %}{% lucide "x" size=14 %} No{% endif %}
</button>
</form>
</td>
<td class="opacity-70">{{ admin.last_login|date:"j M Y"|default:"Never" }}</td>
<td class="text-right">
<form method="post" action="{% url 'controlpanel:admin_revoke' admin.pk %}">
{% csrf_token %}
<button class="btn btn-ghost btn-xs gap-1 text-error" type="submit">{% lucide "user-minus" size=14 %} Revoke</button>
</form>
<button class="btn btn-error btn-outline btn-sm gap-1" type="button" onclick="document.getElementById('{{ admin.pk|dom_id:"admin_revoke_modal" }}').showModal()">{% lucide "user-minus" size=14 %} Revoke</button>
</td>
</tr>
{% empty %}
@@ -68,6 +69,12 @@
</tbody>
</table>
</div>
{% comment %} Dialogs live outside the table: <tbody> may only contain <tr> elements. {% endcomment %}
{% for admin in admins %}
{% url 'controlpanel:admin_revoke' admin.pk as admin_revoke_url %}
{% include "controlpanel/_confirm_modal.html" with modal_id=admin.pk|dom_id:"admin_revoke_modal" title="Revoke platform access" body="Revoke platform access for "|add:admin.email|add:"? They will no longer be able to reach the control panel." action_url=admin_revoke_url submit_label="Revoke" submit_icon="user-minus" %}
{% endfor %}
</div>
</div>
{% endblock panel %}

22
controlpanel/templates/controlpanel/club_admin_form.html
View File

@@ -1,22 +0,0 @@
{% extends "controlpanel/base.html" %}
{% load lucide %}
{% block heading %}Add an admin to {{ club }}{% endblock heading %}
{% block panel %}
<div class="card max-w-xl bg-base-100 shadow">
<div class="card-body">
<div class="alert alert-info">
<span>A club admin can manage everything in this club. They will be required to set up two-factor authentication before they can sign in.</span>
</div>
<form method="post">
{% csrf_token %}
{% include "controlpanel/_form_fields.html" %}
<div class="card-actions justify-end pt-2">
<a class="btn btn-outline gap-2" href="{% url 'controlpanel:club_detail' club.pk %}">{% lucide "arrow-left" size=16 %} Cancel</a>
<button class="btn btn-primary" type="submit">Grant admin</button>
</div>
</form>
</div>
</div>
{% endblock panel %}

19
controlpanel/templates/controlpanel/features.html
View File

@@ -4,10 +4,13 @@
{% block heading %}Features{% endblock heading %}
{% block actions %}
<a class="btn btn-primary gap-2" href="{% url 'controlpanel:flag_create' %}">{% lucide "plus" size=16 %} New feature</a>
<button class="btn btn-primary gap-2" type="button" onclick="document.getElementById('flag_create_modal').showModal()">{% lucide "plus" size=16 %} New feature</button>
{% endblock actions %}
{% block panel %}
{% url 'controlpanel:flag_create' as flag_create_url %}
{% include "controlpanel/_modal_form.html" with modal_id="flag_create_modal" title="New feature" form=flag_form action_url=flag_create_url submit_label="Create" submit_icon="plus" %}
{% comment %}
The lock-down. Clubs get a maintenance page, the scheduled jobs stand down, and the
control panel and the auth screens stay open — otherwise you could not sign in to
@@ -84,13 +87,13 @@
{% elif flag.everyone is False %}
<span class="badge badge-error">Off everywhere</span>
{% else %}
<span class="badge badge-ghost">Per club</span>
<span class="badge badge-info">Per club</span>
{% endif %}
</td>
<td>{{ flag.clubs.count }}</td>
<td class="max-w-xs truncate opacity-70">{{ flag.note|default:"—" }}</td>
<td class="max-w-xs truncate opacity-70">{{ flag.note|default:"-" }}</td>
<td class="text-right">
<a class="btn btn-ghost btn-xs gap-1" href="{% url 'controlpanel:flag_update' flag.pk %}">{% lucide "pencil" size=14 %} Edit</a>
<button class="btn btn-outline btn-sm gap-1" type="button" onclick="document.getElementById('{{ flag.pk|dom_id:"flag_edit_modal" }}').showModal()">{% lucide "pencil" size=14 %} Edit</button>
</td>
</tr>
{% empty %}
@@ -101,6 +104,12 @@
</tbody>
</table>
</div>
{% comment %} Dialogs live outside the table: <tbody> may only contain <tr> elements. {% endcomment %}
{% for flag in flags %}
{% url 'controlpanel:flag_update' flag.pk as flag_update_url %}
{% include "controlpanel/_modal_form.html" with modal_id=flag.pk|dom_id:"flag_edit_modal" title="Edit "|add:flag.name form=flag.edit_form action_url=flag_update_url submit_label="Save" submit_icon="check" %}
{% endfor %}
</div>
</div>
<div class="card bg-base-100 shadow">
@@ -113,7 +122,7 @@
{% for switch in switches %}
<tr>
<td class="font-mono font-medium">{{ switch.name }}</td>
<td class="opacity-70">{{ switch.note|default:"—" }}</td>
<td class="opacity-70">{{ switch.note|default:"-" }}</td>
<td class="text-right">
<form method="post" action="{% url 'controlpanel:switch_toggle' switch.pk %}">
{% csrf_token %}

19
controlpanel/templates/controlpanel/flag_form.html
View File

@@ -1,19 +0,0 @@
{% extends "controlpanel/base.html" %}
{% load lucide %}
{% block heading %}{% if object %}Edit {{ object.name }}{% else %}New feature{% endif %}{% endblock heading %}
{% block panel %}
<div class="card max-w-xl bg-base-100 shadow">
<div class="card-body">
<form method="post">
{% csrf_token %}
{% include "controlpanel/_form_fields.html" %}
<div class="card-actions justify-end pt-2">
<a class="btn btn-outline gap-2" href="{% url 'controlpanel:features' %}">{% lucide "arrow-left" size=16 %} Cancel</a>
<button class="btn btn-primary" type="submit">Save</button>
</div>
</form>
</div>
</div>
{% endblock panel %}

7
controlpanel/templatetags/ui.py
View File

@@ -32,10 +32,11 @@ def as_alert(message):
"""Presentation for one Django message: icon, bold title, body, colour.
Django messages carry a level and a string — there is no title field — so the
title comes from the level, and a call site that wants a specific one passes it
as ``extra_tags``::
title comes from the level, unless the message carries one as ``extra_tags``.
Call sites queue messages with ``notify`` (controlpanel/messages.py), which sets
exactly that from a compact ``"<level>|<title>|<body>"`` spec::
messages.success(request, f"{club} is live.", extra_tags="Club created")
notify(request, f"s|Club created|{club} is live.")
Keyed on ``level_tag``, never ``tags``: ``tags`` is extra_tags and level_tag
joined, so a message carrying a custom title would stop matching its own level

91
controlpanel/tests.py
View File

@@ -9,8 +9,9 @@ from django.conf import settings
from django.contrib import messages
from django.contrib.auth import get_user_model
from django.contrib.messages.storage.base import Message
from django.contrib.messages.storage.fallback import FallbackStorage
from django.core.cache import cache
from django.test import TestCase, override_settings
from django.test import RequestFactory, TestCase, override_settings
from django.urls import reverse
from django.utils import timezone
from waffle import get_waffle_flag_model, get_waffle_switch_model
@@ -25,6 +26,7 @@ from members.models import Member
from shop.models import Order
from teams.models import Position, StaffAssignment, Team, TeamMembership
from .messages import LEVELS, notify
from .services.admins import grant_club_admin
from .services.platform_admins import PlatformAdminError, is_last_superuser, set_platform_access
from .services.statistics import (
@@ -175,11 +177,18 @@ class ClubAdminManagementTests(ControlPanelTestBase):
self.assertEqual(ClubRole.objects.get(club=self.club, member__user=user).role, ClubRole.Roles.ADMIN)
def test_a_new_email_must_come_with_a_name(self):
response = self.add_admin(email="nameless@example.com", first_name="", last_name="")
# Reachable only via the "Add admin" modal on the club detail page, so a rejected
# submission bounces back there with the error as a message.
response = self.client.post(reverse("controlpanel:club_admin_add", args=[self.club.pk]), {"email": "nameless@example.com", "first_name": "", "last_name": ""}, follow=True)
self.assertEqual(response.status_code, 200)
self.assertRedirects(response, reverse("controlpanel:club_detail", args=[self.club.pk]))
self.assertFalse(ClubRole.objects.exists())
self.assertFormError(response.context["form"], "first_name", "Required: this email has no account yet.")
self.assertContains(response, "Required: this email has no account yet.")
def test_add_admin_is_post_only(self):
response = self.client.get(reverse("controlpanel:club_admin_add", args=[self.club.pk]))
self.assertEqual(response.status_code, 405)
def test_an_existing_member_is_promoted_rather_than_duplicated(self):
user = User.objects.create_user(email="existing@example.com", password="pw-secret-123")
@@ -315,8 +324,10 @@ class PlatformAdminTests(TestCase):
def test_superuser_sees_the_admins_section(self):
self.assertEqual(self.client.get(reverse("controlpanel:admins")).status_code, 200)
def test_the_grant_form_renders(self):
self.assertEqual(self.client.get(reverse("controlpanel:admin_add")).status_code, 200)
def test_the_grant_form_is_post_only(self):
# Reachable only via the "Grant access" modal on the admins page: there is no
# standalone template to render on a GET.
self.assertEqual(self.client.get(reverse("controlpanel:admin_add")).status_code, 405)
def test_grant_access_to_a_new_email_creates_a_staff_account(self):
self.client.post(reverse("controlpanel:admin_add"), {"email": "New.Admin@Example.com"})
@@ -407,9 +418,17 @@ class FeatureViewTests(ControlPanelTestBase):
self.assertContains(response, "shop")
self.assertContains(response, "maintenance")
def test_the_flag_forms_render(self):
self.assertEqual(self.client.get(reverse("controlpanel:flag_create")).status_code, 200)
self.assertEqual(self.client.get(reverse("controlpanel:flag_update", args=[self.flag.pk])).status_code, 200)
def test_the_flag_forms_are_post_only(self):
# Reachable only through a modal on the features page: there is no standalone
# template to render on a GET.
self.assertEqual(self.client.get(reverse("controlpanel:flag_create")).status_code, 405)
self.assertEqual(self.client.get(reverse("controlpanel:flag_update", args=[self.flag.pk])).status_code, 405)
def test_an_invalid_flag_submission_redirects_with_a_message(self):
response = self.client.post(reverse("controlpanel:flag_create"), {"name": "", "note": "", "percent": "", "everyone": ""}, follow=True)
self.assertRedirects(response, reverse("controlpanel:features"))
self.assertContains(response, "This field is required")
def test_create_a_flag(self):
self.client.post(reverse("controlpanel:flag_create"), {"name": "news", "note": "News module", "percent": "", "everyone": ""})
@@ -456,33 +475,75 @@ class FeatureViewTests(ControlPanelTestBase):
self.assertNotContains(response, reverse("controlpanel:club_feature_toggle", args=[self.club.pk, self.flag.pk]))
class NotifyTests(TestCase):
def request(self):
request = RequestFactory().get("/")
request.session = {}
storage = FallbackStorage(request)
request._messages = storage
return request, storage
def test_splits_level_title_and_body(self):
request, storage = self.request()
notify(request, "s|Club created|Ajax United is live.")
[message] = list(storage)
self.assertEqual(message.level, messages.SUCCESS)
self.assertEqual(message.extra_tags, "Club created")
self.assertEqual(message.message, "Ajax United is live.")
def test_maps_every_level_code_to_its_django_level(self):
self.assertEqual(LEVELS, {"s": messages.SUCCESS, "i": messages.INFO, "w": messages.WARNING, "e": messages.ERROR, "d": messages.DEBUG})
def test_a_pipe_inside_the_body_is_preserved_intact(self):
# maxsplit=2 stops after the level and the title, so a "|" a club/tier/flag name
# might contain stays part of the body rather than truncating it.
request, storage = self.request()
notify(request, "s|Title|Before | after.")
[message] = list(storage)
self.assertEqual(message.message, "Before | after.")
def test_an_empty_title_falls_back_to_the_generic_one_at_render_time(self):
request, storage = self.request()
notify(request, "s||No custom title.")
[message] = list(storage)
self.assertEqual(as_alert(message)["title"], "Done")
class MessageAlertTests(TestCase):
def alert(self, level, text, extra_tags=None):
return as_alert(Message(level, text, extra_tags=extra_tags))
def test_each_level_gets_its_own_icon_title_and_colour(self):
self.assertEqual(self.alert(messages.SUCCESS, "Saved.")["icon"], "circle-check")
self.assertEqual(self.alert(messages.WARNING, "Careful.")["css"], "alert-warning")
self.assertEqual(self.alert(messages.WARNING, "Careful.")["css"], "alert-warning border-warning")
self.assertEqual(self.alert(messages.ERROR, "Boom.")["title"], "Something went wrong")
self.assertEqual(self.alert(messages.INFO, "FYI.")["css"], "alert-info")
self.assertEqual(self.alert(messages.INFO, "FYI.")["css"], "alert-info border-info")
def test_extra_tags_override_the_title(self):
alert = self.alert(messages.SUCCESS, "Ajax United is live.", extra_tags="Club created")
self.assertEqual(alert["title"], "Club created")
self.assertEqual(alert["body"], "Ajax United is live.")
self.assertEqual(alert["css"], "alert-success") # a custom title must not change the level
self.assertEqual(alert["css"], "alert-success border-success") # a custom title must not change the level
def test_an_unknown_level_falls_back_to_info(self):
self.assertEqual(self.alert(999, "Odd.")["css"], "alert-info")
self.assertEqual(self.alert(999, "Odd.")["css"], "alert-info border-info")
class MessageRenderingTests(ControlPanelTestBase):
def test_a_message_renders_as_a_soft_alert_with_icon_and_title(self):
# club_archive queues its message through `notify`, which sets a custom title —
# so the generic per-level one ("Careful") must not show.
response = self.client.post(reverse("controlpanel:club_archive", args=[self.club.pk]), follow=True)
self.assertContains(response, "alert alert-soft alert-warning")
self.assertContains(response, '<div class="font-bold">Careful</div>', html=False)
self.assertContains(response, "alert alert-soft border-2 alert-warning border-warning")
self.assertContains(response, '<div class="font-bold">Club archived</div>', html=False)
self.assertContains(response, "<svg") # the lucide icon

127
controlpanel/views.py
View File

@@ -1,6 +1,5 @@
from contextlib import contextmanager
from django.contrib import messages
from django.contrib.auth import get_user_model
from django.db.models import Count
from django.http import HttpResponse
@@ -19,6 +18,7 @@ from club.models import Club, ClubRole
from features.models import Maintenance
from .forms import ClubAdminForm, ClubForm, DuePaymentForm, FlagForm, MaintenanceForm, OpenPeriodForm, PlatformAdminForm, SubscriptionForm, TierForm, TierPriceForm
from .messages import notify
from .mixins import PlatformStaffRequiredMixin, PlatformSuperuserRequiredMixin, RedirectOnInvalidMixin
from .services.admins import grant_club_admin, revoke_club_admin
from .services.platform_admins import (
@@ -35,7 +35,7 @@ Switch = get_waffle_switch_model()
@contextmanager
def suppress_billing_errors(request):
def suppress_billing_errors(request, title="Billing error"):
"""Turn a BillingError into an error message rather than letting it propagate.
Fits call sites that fall through to the same redirect on the happy and unhappy path
@@ -45,7 +45,7 @@ def suppress_billing_errors(request):
try:
yield
except BillingError as error:
messages.error(request, str(error))
notify(request, f"e|{title}|{error}")
class DashboardView(PlatformStaffRequiredMixin, TemplateView):
@@ -91,7 +91,7 @@ class ClubCreateView(PlatformStaffRequiredMixin, CreateView):
def form_valid(self, form):
response = super().form_valid(form)
messages.success(self.request, f"Club “{self.object}” created.")
notify(self.request, f"s|Club created|Club “{self.object}” created.")
return response
def get_success_url(self):
@@ -108,7 +108,7 @@ class ClubUpdateView(PlatformStaffRequiredMixin, UpdateView):
def form_valid(self, form):
response = super().form_valid(form)
messages.success(self.request, f"Club “{self.object}” updated.")
notify(self.request, f"s|Club updated|Club “{self.object}” updated.")
return response
def get_success_url(self):
@@ -143,6 +143,7 @@ class ClubDetailView(PlatformStaffRequiredMixin, DetailView):
dues=dues,
today=timezone.localdate(),
admins=ClubRole.objects.filter(club=self.object, role=ClubRole.Roles.ADMIN).select_related("member", "member__user"),
admin_form=ClubAdminForm(),
flags=flags_for_club(self.object),
open_period_form=OpenPeriodForm(),
open_period_blurb=f"Next period starts {date_format(next_start, 'j M Y')} unless you say otherwise. By default it continues from the end of the last one, so a lapsed year is still owed — pick a start date to forgive the gap.",
@@ -157,7 +158,7 @@ class ClubArchiveView(PlatformStaffRequiredMixin, View):
def post(self, request, pk):
club = get_object_or_404(Club, pk=pk)
club.archive()
messages.warning(request, f"Club “{club}” archived. Its subdomain no longer resolves.")
notify(request, f"w|Club archived|Club “{club}” archived. Its subdomain no longer resolves.")
return redirect("controlpanel:club_detail", pk=club.pk)
@@ -165,24 +166,28 @@ class ClubRestoreView(PlatformStaffRequiredMixin, View):
def post(self, request, pk):
club = get_object_or_404(Club, pk=pk)
club.restore()
messages.success(request, f"Club “{club}” restored.")
notify(request, f"s|Club restored|Club “{club}” restored.")
return redirect("controlpanel:club_detail", pk=club.pk)
class ClubAdminAddView(PlatformStaffRequiredMixin, FormView):
class ClubAdminAddView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, FormView):
"""Reachable only via the "Add admin" modal on the club detail page — POST-only, and
there is no standalone template to render on GET or on a rejected submission."""
form_class = ClubAdminForm
template_name = "controlpanel/club_admin_form.html"
http_method_names = ["post"]
invalid_redirect_url_name = "controlpanel:club_detail"
@property
def club(self):
return get_object_or_404(Club, pk=self.kwargs["pk"])
def get_context_data(self, **kwargs):
return super().get_context_data(nav="clubs", club=self.club, **kwargs)
def get_invalid_redirect_kwargs(self):
return {"pk": self.kwargs["pk"]}
def form_valid(self, form):
role = grant_club_admin(self.club, **form.cleaned_data)
messages.success(self.request, f"{role.member} is now an admin of {role.club}. They must set up two-factor authentication before they can sign in.")
notify(self.request, f"s|Admin added|{role.member} is now an admin of {role.club}. They must set up two-factor authentication before they can sign in.")
return redirect("controlpanel:club_detail", pk=self.kwargs["pk"])
@@ -191,7 +196,7 @@ class ClubAdminRemoveView(PlatformStaffRequiredMixin, View):
role = get_object_or_404(ClubRole, pk=role_pk, club_id=pk, role=ClubRole.Roles.ADMIN)
member = role.member
revoke_club_admin(role)
messages.warning(request, f"{member} is no longer an admin of this club.")
notify(request, f"w|Admin removed|{member} is no longer an admin of this club.")
return redirect("controlpanel:club_detail", pk=pk)
@@ -204,10 +209,10 @@ class ClubFeatureToggleView(PlatformStaffRequiredMixin, View):
if flag.clubs.filter(pk=club.pk).exists():
flag.clubs.remove(club)
messages.warning(request, f"“{flag.name}” turned off for {club}.")
notify(request, f"w|Feature disabled|“{flag.name}” turned off for {club}.")
else:
flag.clubs.add(club)
messages.success(request, f"“{flag.name}” turned on for {club}.")
notify(request, f"s|Feature enabled|“{flag.name}” turned on for {club}.")
return redirect("controlpanel:club_detail", pk=club.pk)
@@ -216,9 +221,16 @@ class FeatureListView(PlatformStaffRequiredMixin, TemplateView):
template_name = "controlpanel/features.html"
def get_context_data(self, **kwargs):
# Bound per-row so each flag's "Edit" modal can render its own form: the template
# can't call FlagForm(instance=flag) itself, so the form rides along on the flag.
flags = list(Flag.objects.prefetch_related("clubs").order_by("name"))
for flag in flags:
flag.edit_form = FlagForm(instance=flag)
return super().get_context_data(
nav="features",
flags=Flag.objects.prefetch_related("clubs").order_by("name"),
flags=flags,
flag_form=FlagForm(),
switches=Switch.objects.order_by("name"),
maintenance=Maintenance.current(),
maintenance_form=MaintenanceForm(),
@@ -232,45 +244,46 @@ class MaintenanceView(PlatformStaffRequiredMixin, View):
def post(self, request):
if Maintenance.is_on():
Maintenance.stop()
messages.success(request, "Maintenance ended. The clubs are back.")
notify(request, "s|Maintenance ended|The clubs are back.")
else:
form = MaintenanceForm(request.POST)
message = form.cleaned_data["message"] if form.is_valid() else ""
Maintenance.start(message=message, user=request.user)
messages.warning(request, "Platform closed. Every club subdomain now serves a maintenance page, and the scheduled jobs stand down.")
notify(request, "w|Platform closed|Every club subdomain now serves a maintenance page, and the scheduled jobs stand down.")
return redirect("controlpanel:features")
class FlagCreateView(PlatformStaffRequiredMixin, CreateView):
class FlagCreateView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, CreateView):
"""Reachable only via the "New feature" modal on the features page — POST-only, and
there is no standalone template to render on GET or on a rejected submission."""
model = Flag
form_class = FlagForm
template_name = "controlpanel/flag_form.html"
success_url = None
def get_context_data(self, **kwargs):
return super().get_context_data(nav="features", **kwargs)
http_method_names = ["post"]
invalid_redirect_url_name = "controlpanel:features"
def form_valid(self, form):
response = super().form_valid(form)
messages.success(self.request, f"Feature “{self.object.name}” created.")
notify(self.request, f"s|Feature created|Feature “{self.object.name}” created.")
return response
def get_success_url(self):
return reverse("controlpanel:features")
class FlagUpdateView(PlatformStaffRequiredMixin, UpdateView):
class FlagUpdateView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, UpdateView):
"""Reachable only via a flag's "Edit" modal on the features page — POST-only, and
there is no standalone template to render on GET or on a rejected submission."""
model = Flag
form_class = FlagForm
template_name = "controlpanel/flag_form.html"
def get_context_data(self, **kwargs):
return super().get_context_data(nav="features", **kwargs)
http_method_names = ["post"]
invalid_redirect_url_name = "controlpanel:features"
def form_valid(self, form):
response = super().form_valid(form)
messages.success(self.request, f"Feature “{self.object.name}” updated.")
notify(self.request, f"s|Feature updated|Feature “{self.object.name}” updated.")
return response
def get_success_url(self):
@@ -284,7 +297,8 @@ class SwitchToggleView(PlatformStaffRequiredMixin, View):
switch = get_object_or_404(Switch, pk=pk)
switch.active = not switch.active
switch.save()
messages.success(request, f"Switch “{switch.name}” is now {'on' if switch.active else 'off'}.")
title = "Switch on" if switch.active else "Switch off"
notify(request, f"s|{title}|Switch “{switch.name}” is now {'on' if switch.active else 'off'}.")
return redirect("controlpanel:features")
@@ -292,19 +306,20 @@ class PlatformAdminListView(PlatformSuperuserRequiredMixin, TemplateView):
template_name = "controlpanel/admins.html"
def get_context_data(self, **kwargs):
return super().get_context_data(nav="admins", admins=platform_admins(), **kwargs)
return super().get_context_data(nav="admins", admins=platform_admins(), admin_form=PlatformAdminForm(), **kwargs)
class PlatformAdminAddView(PlatformSuperuserRequiredMixin, FormView):
class PlatformAdminAddView(PlatformSuperuserRequiredMixin, RedirectOnInvalidMixin, FormView):
"""Reachable only via the "Grant access" modal on the admins page — POST-only, and
there is no standalone template to render on GET or on a rejected submission."""
form_class = PlatformAdminForm
template_name = "controlpanel/admin_form.html"
def get_context_data(self, **kwargs):
return super().get_context_data(nav="admins", **kwargs)
http_method_names = ["post"]
invalid_redirect_url_name = "controlpanel:admins"
def form_valid(self, form):
user = grant_platform_access(form.cleaned_data["email"], is_superuser=form.cleaned_data["is_superuser"])
messages.success(self.request, f"{user.email} now has platform access. They must set up two-factor authentication before they can sign in.")
notify(self.request, f"s|Platform access granted|{user.email} now has platform access. They must set up two-factor authentication before they can sign in.")
return redirect("controlpanel:admins")
@@ -319,9 +334,9 @@ class PlatformAdminUpdateView(PlatformSuperuserRequiredMixin, View):
is_superuser=request.POST.get("is_superuser") == "1",
)
except PlatformAdminError as error:
messages.error(request, str(error))
notify(request, f"e|Couldn't update access|{error}")
else:
messages.success(request, f"Updated platform access for {user.email}.")
notify(request, f"s|Access updated|Updated platform access for {user.email}.")
return redirect("controlpanel:admins")
@@ -331,9 +346,9 @@ class PlatformAdminRevokeView(PlatformSuperuserRequiredMixin, View):
try:
revoke_platform_access(request.user, user)
except PlatformAdminError as error:
messages.error(request, str(error))
notify(request, f"e|Couldn't revoke access|{error}")
else:
messages.warning(request, f"{user.email} no longer has platform access.")
notify(request, f"w|Access revoked|{user.email} no longer has platform access.")
return redirect("controlpanel:admins")
@@ -377,7 +392,7 @@ class TierCreateView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, CreateV
invalid_redirect_url_name = "controlpanel:billing"
def get_success_url(self):
messages.success(self.request, f"Tier “{self.object}” created. Give it a price before billing anyone.")
notify(self.request, f"s|Plan created|Tier “{self.object}” created. Give it a price before billing anyone.")
return reverse("controlpanel:billing")
@@ -391,7 +406,7 @@ class TierUpdateView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, UpdateV
invalid_redirect_url_name = "controlpanel:billing"
def get_success_url(self):
messages.success(self.request, f"Tier “{self.object}” updated.")
notify(self.request, f"s|Plan updated|Tier “{self.object}” updated.")
return reverse("controlpanel:billing")
@@ -415,7 +430,7 @@ class TierPriceCreateView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, Cr
def form_valid(self, form):
form.instance.tier = self.tier
response = super().form_valid(form)
messages.success(self.request, f"{self.tier} is €{self.object.amount} for periods opening from {self.object.active_from}.")
notify(self.request, f"s|Price added|{self.tier} is €{self.object.amount} for periods opening from {self.object.active_from}.")
return response
def get_success_url(self):
@@ -450,17 +465,17 @@ class SubscribeClubView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, Form
def form_valid(self, form):
club = self.club
existing = getattr(club, "subscription", None)
with suppress_billing_errors(self.request):
with suppress_billing_errors(self.request, title="Couldn't change plan"):
if existing:
# Changing tier does not re-bill: the current period keeps the amount it was
# issued at, and the new rate applies from the next one.
subscription = form.save(commit=False)
subscription.club = club
subscription.save()
messages.success(self.request, f"{club} is now on {subscription.tier}. The current period keeps the amount it was billed at.")
notify(self.request, f"s|Plan changed|{club} is now on {subscription.tier}. The current period keeps the amount it was billed at.")
else:
subscribe(club, form.cleaned_data["tier"], start=form.cleaned_data.get("start"), auto_archive=form.cleaned_data["auto_archive"], auto_renew=form.cleaned_data["auto_renew"])
messages.success(self.request, f"{club} is on {form.cleaned_data['tier']}. Its first period is open.")
notify(self.request, f"s|Billing started|{club} is on {form.cleaned_data['tier']}. Its first period is open.")
return redirect("controlpanel:club_detail", pk=club.pk)
@@ -483,7 +498,7 @@ class RecordPaymentView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, Form
def form_valid(self, form):
due = self.due
with suppress_billing_errors(self.request):
with suppress_billing_errors(self.request, title="Couldn't record payment"):
record_payment(
due,
form.cleaned_data["amount"],
@@ -494,7 +509,7 @@ class RecordPaymentView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, Form
user=self.request.user,
)
due.refresh_from_db()
messages.success(self.request, f"€{form.cleaned_data['amount']} recorded. {due.get_status_display().capitalize()} — €{due.balance} outstanding.")
notify(self.request, f"s|Payment recorded|€{form.cleaned_data['amount']} recorded. {due.get_status_display().capitalize()} — €{due.balance} outstanding.")
return redirect("controlpanel:club_detail", pk=due.club_id)
@@ -502,9 +517,9 @@ class RecordPaymentView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, Form
class WaiveDueView(PlatformStaffRequiredMixin, View):
def post(self, request, pk):
due = get_object_or_404(Due, pk=pk)
with suppress_billing_errors(request):
with suppress_billing_errors(request, title="Couldn't waive period"):
waive(due)
messages.warning(request, f"Period {due.period_start} to {due.period_end} waived. Nothing is owed and the club will not be archived for it.")
notify(request, f"w|Period waived|Period {due.period_start} to {due.period_end} waived. Nothing is owed and the club will not be archived for it.")
return redirect("controlpanel:club_detail", pk=due.club_id)
@@ -530,9 +545,9 @@ class OpenPeriodView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, FormVie
def form_valid(self, form):
club = self.club
start = form.cleaned_data.get("start")
with suppress_billing_errors(self.request):
with suppress_billing_errors(self.request, title="Couldn't open period"):
due = reactivate(club, start=start) if club.is_archived else open_period(club, start=start)
messages.success(self.request, f"Period {due.period_start} to {due.period_end} opened for €{due.amount}. Invoice {due.invoice.number}.")
notify(self.request, f"s|Period opened|Period {due.period_start} to {due.period_end} opened for €{due.amount}. Invoice {due.invoice.number}.")
return redirect("controlpanel:club_detail", pk=club.pk)
@@ -545,7 +560,7 @@ class InvoicePdfView(PlatformStaffRequiredMixin, View):
pdf = invoice_pdf(invoice)
except BillingError as error:
# The native PDF libraries are missing: say so rather than 500.
messages.error(request, str(error))
notify(request, f"e|PDF unavailable|{error}")
return redirect("controlpanel:club_detail", pk=due.club_id)
response = HttpResponse(pdf, content_type="application/pdf")

57
formbuilder/services/submission.py
View File

@@ -3,13 +3,13 @@
from django.db import transaction
from django.utils import timezone
from formbuilder.models import Answer, Field, Submission
from formbuilder.models import Answer, Submission
from .options import allowed_values
from .form_factory import build_form
class FormSubmissionError(Exception):
"""Raised when a submission is rejected. ``errors`` maps field key -> message."""
"""Raised when a submission is rejected. ``errors`` maps field key -> messages."""
def __init__(self, message, *, errors=None):
super().__init__(message)
@@ -21,12 +21,17 @@ def _is_empty(value):
@transaction.atomic
def submit_form(form, member, data, *, when=None):
"""Create a Submission (with Answers) for ``data`` or raise FormSubmissionError."""
def submit_form(form, member, data, *, files=None, when=None):
"""Create a Submission (with Answers) for ``data``/``files`` or raise FormSubmissionError.
Validation goes through ``build_form`` — the same dynamic Django Form the UI would
render — so a NUMBER field is actually checked as a decimal, an EMAIL as an email, a
CHOICE against its real options, and so on, rather than a hand-rolled subset of that.
"""
when = when or timezone.now()
_check_open(form, member, when)
cleaned = _clean_answers(form, data)
cleaned = _clean_answers(form, data, files)
submission = Submission.objects.create(form=form, member=member)
Answer.objects.bulk_create([Answer(submission=submission, field=field, value=value) for field, value in cleaned])
@@ -48,37 +53,13 @@ def _check_open(form, member, when):
raise FormSubmissionError("You have reached the maximum number of submissions for this form.")
def _clean_answers(form, data):
errors = {}
cleaned = []
for field in form.fields.filter(is_active=True):
raw = data.get(field.key)
if _is_empty(raw):
if field.required:
errors[field.key] = "This field is required."
continue
message = _validate_choice(field, raw)
if message is not None:
errors[field.key] = message
continue
cleaned.append((field, raw))
if errors:
def _clean_answers(form, data, files):
bound_form = build_form(form, data=data, files=files or {})
if not bound_form.is_valid():
errors = {key: list(messages) for key, messages in bound_form.errors.items()}
raise FormSubmissionError("The submission has errors.", errors=errors)
return cleaned
def _validate_choice(field, raw):
if field.field_type == Field.FieldType.CHOICE:
allowed = allowed_values(field)
if allowed and raw not in allowed:
return "Select a valid choice."
elif field.field_type == Field.FieldType.MULTICHOICE:
allowed = allowed_values(field)
values = raw if isinstance(raw, list) else [raw]
if allowed and not set(values) <= allowed:
return "Select valid choices."
return None
# Blank optional answers are validated (they may legitimately be empty) but not
# stored — an Answer row exists only where the submitter actually said something.
fields_by_key = {field.key: field for field in form.fields.filter(is_active=True)}
return [(fields_by_key[key], value) for key, value in bound_form.cleaned_data.items() if not _is_empty(value)]

18
formbuilder/tests.py
View File

@@ -211,6 +211,24 @@ class SubmitFormTests(FormbuilderTestBase):
self.assertIn("size", ctx.exception.errors)
def test_number_field_rejects_non_numeric_input(self):
# Validation goes through the same dynamic Django Form the UI renders, so a
# NUMBER field is checked as a decimal — not merely "present".
Field.objects.create(form=self.form, key="age", label="Age", field_type=Field.FieldType.NUMBER, required=True, order=3)
with self.assertRaises(FormSubmissionError) as ctx:
submit_form(self.form, self.member, {"name": "Jane", "age": "not-a-number"})
self.assertIn("age", ctx.exception.errors)
def test_email_field_rejects_an_invalid_address(self):
Field.objects.create(form=self.form, key="contact", label="Contact", field_type=Field.FieldType.EMAIL, required=True, order=3)
with self.assertRaises(FormSubmissionError) as ctx:
submit_form(self.form, self.member, {"name": "Jane", "contact": "not-an-email"})
self.assertIn("contact", ctx.exception.errors)
def test_multichoice_validation(self):
field = Field.objects.create(form=self.form, key="days", label="Days", field_type=Field.FieldType.MULTICHOICE, required=False, order=3, options=["mon", "tue", "wed"])

15
members/services/member_csv_importer.py
View File

@@ -123,7 +123,7 @@ class MemberCsvImporter:
},
)
club, _ = Club.objects.get_or_create(name=club_name)
club = self.get_club(club_name)
season = self.get_current_season(club)
_, membership_created = ClubMembership.objects.update_or_create(
@@ -141,6 +141,19 @@ class MemberCsvImporter:
membership_created=membership_created,
)
def get_club(self, club_name) -> Club:
# Never get_or_create: Club.name isn't unique, so a typo'd or differently-cased
# value would otherwise either spin up a duplicate club or raise
# MultipleObjectsReturned against one that already exists. Matching
# case-insensitively absorbs the harmless variety (a CSV export's casing rarely
# matches the platform's own); an unknown club is a data problem the importer
# must not paper over by inventing one.
club = Club.objects.filter(name__iexact=club_name).first()
if club is None:
raise ValueError(f"Unknown club '{club_name}'.")
return club
def get_current_season(self, club) -> Season:
# Season.get_current() is tenant-scoped, so bind the row's club as the
# active tenant for the lookup.

45
members/tests.py
View File

@@ -554,8 +554,9 @@ class ImportMembersCsvCommandTests(TestCase):
self.assertFalse(Member.objects.filter(email="nofirst@example.com").exists())
def test_import_skips_row_when_club_has_no_current_season(self):
# "New Club" has no season, so the row is skipped and — because the row
# is atomic — the member and club creation roll back too.
# The club exists but has no season, so the row is skipped and — because
# the row is atomic — the member creation rolls back too.
Club.objects.create(name="New Club")
csv_path = self.write_csv(
"\n".join(
[
@@ -571,7 +572,45 @@ class ImportMembersCsvCommandTests(TestCase):
self.assertIn("No current season for club 'New Club'.", stderr)
self.assertIn("Rows skipped: 1.", stdout)
self.assertFalse(Member.objects.filter(email="jane@example.com").exists())
self.assertFalse(Club.objects.filter(name="New Club").exists())
def test_import_skips_row_for_an_unknown_club(self):
# The importer must not silently spin up a club for a typo'd or unknown
# name — that is a data problem, not something to paper over.
csv_path = self.write_csv(
"\n".join(
[
"first_name,last_name,email,date_of_birth,create_account,club_name,license_number",
"Jane,Doe,jane@example.com,2010-04-12,false,Nonexistent Club,LIC-001",
]
)
)
stdout, stderr = self.call_import_command(csv_path)
self.assertIn("Row 2 skipped:", stderr)
self.assertIn("Unknown club 'Nonexistent Club'.", stderr)
self.assertIn("Rows skipped: 1.", stdout)
self.assertFalse(Member.objects.filter(email="jane@example.com").exists())
self.assertFalse(Club.objects.filter(name="Nonexistent Club").exists())
def test_import_matches_a_club_name_case_insensitively(self):
# A CSV export's casing rarely matches the platform's own; that is
# harmless variation, not a different club.
csv_path = self.write_csv(
"\n".join(
[
"first_name,last_name,email,date_of_birth,create_account,club_name,license_number",
"Jane,Doe,jane@example.com,2010-04-12,false,city swim club,LIC-001",
]
)
)
stdout, stderr = self.call_import_command(csv_path)
self.assertEqual(stderr, "")
self.assertIn("Rows skipped: 0.", stdout)
membership = ClubMembership.objects.get(club=self.club, member__email="jane@example.com")
self.assertEqual(membership.season, self.season)
class MemberImportResultTests(TestCase):

22
shop/models.py
View File

@@ -11,6 +11,14 @@ from rosterchief.base import ClubScopedModel, UUIDModel, validate_club_scope
from teams.models import Position, Team
class DiscountType(models.TextChoices):
"""Shared by ``Product`` (early-bird), ``Discount`` and ``AppliedDiscount`` (its
snapshot on an order) — one discount vocabulary, not three copies of it."""
PERCENTAGE = "percentage", _("Percentage")
FIXED_AMOUNT = "fixed_amount", _("Fixed amount")
def next_scoped_number(instance, code):
"""Next per-club sequential number for the current year: ``<code>-<year>-<seq>``."""
prefix = f"{code}-{timezone.now().year}-"
@@ -46,10 +54,6 @@ class Product(ClubScopedModel):
MERCHANDISE = "merchandise", _("Merchandise")
DONATION = "donation", _("Donation")
class DiscountType(models.TextChoices):
PERCENTAGE = "percentage", _("Percentage")
FIXED_AMOUNT = "fixed_amount", _("Fixed amount")
name = models.CharField(_("name"), max_length=255)
slug = models.SlugField(_("slug"), max_length=255, blank=True)
@@ -191,10 +195,6 @@ class OrderLine(UUIDModel):
class Discount(ClubScopedModel):
class DiscountType(models.TextChoices):
PERCENTAGE = "percentage", _("Percentage")
FIXED_AMOUNT = "fixed_amount", _("Fixed amount")
name = models.CharField(_("name"), max_length=255)
slug = models.SlugField(_("slug"), max_length=255, blank=True)
description = models.TextField(_("description"), blank=True)
@@ -219,10 +219,6 @@ class Discount(ClubScopedModel):
class AppliedDiscount(UUIDModel):
class DiscountType(models.TextChoices):
PERCENTAGE = "percentage", _("Percentage")
FIXED_AMOUNT = "fixed_amount", _("Fixed amount")
order = models.ForeignKey(Order, on_delete=models.CASCADE, related_name="applied_discounts", verbose_name=_("order"))
discount = models.ForeignKey(Discount, on_delete=models.PROTECT, related_name="applied_discounts", verbose_name=_("discount"))
@@ -240,7 +236,7 @@ class AppliedDiscount(UUIDModel):
]
def __str__(self):
suffix = "%" if self.discount_type == self.DiscountType.PERCENTAGE else ""
suffix = "%" if self.discount_type == DiscountType.PERCENTAGE else ""
return f"{self.discount} - {self.discount_amount}{suffix}"
def clean(self):

5
shop/tests.py
View File

@@ -21,6 +21,7 @@ from .models import (
Cart,
CartItem,
Discount,
DiscountType,
Invoice,
Order,
OrderLine,
@@ -251,12 +252,12 @@ class AppliedDiscountTests(ShopEntitiesTestBase):
return AppliedDiscount.objects.create(**kwargs)
def test_str_percentage_shows_percent(self):
applied = self.apply(discount_type=AppliedDiscount.DiscountType.PERCENTAGE)
applied = self.apply(discount_type=DiscountType.PERCENTAGE)
self.assertEqual(str(applied), "Sibling - 10.00%")
def test_str_fixed_amount_has_no_percent(self):
applied = self.apply(discount_type=AppliedDiscount.DiscountType.FIXED_AMOUNT)
applied = self.apply(discount_type=DiscountType.FIXED_AMOUNT)
self.assertEqual(str(applied), "Sibling - 10.00")