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

Honor auto_archive in the reminder email subject and the trial form

Browse Source 
Two gaps found while checking whether auto_archive is honored
end-to-end (archive_overdue_clubs and the on-screen banner already
got it right):

- reminder_subject.txt branched only on notice.level, so a club with
  auto_archive off -- one that will NEVER be archived -- still got
  "Action required: X is about to be archived" as its subject line,
  contradicting the correctly-worded body underneath. Now gated on
  notice.level == 'error' AND notice.will_archive.
- TrialForm had no auto_renew/auto_archive fields at all, so a trial
  could only ever be started on the service defaults (both True).
  The only way to change either afterwards was "Change plan", which
  ends the trial as a side effect. Added both, matching
  SubscriptionForm's existing pair.
This commit is contained in:
Bernard Siebens
2026-08-08 19:14:36 +02:00
parent c2a4380b70
commit 617271f0d0
5 changed files with 60 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
billing/templates/billing/email/reminder_subject.txt
View File

@@ -1 +1 @@
{% load i18n %}{% if notice.level == 'error' %}{% blocktrans with club=club.name %}Action required: {{ club }} is about to be archived{% endblocktrans %}{% else %}{% blocktrans with club=club.name %}Platform fees are due for {{ club }}{% endblocktrans %}{% endif %}
{% load i18n %}{% if notice.level == 'error' and notice.will_archive %}{% blocktrans with club=club.name %}Action required: {{ club }} is about to be archived{% endblocktrans %}{% else %}{% blocktrans with club=club.name %}Platform fees are due for {{ club }}{% endblocktrans %}{% endif %}

36
billing/tests.py
View File

@@ -752,6 +752,42 @@ class BillingReminderTests(BillingTestBase):
def test_a_reminder_goes_to_the_club_admins(self):
self.assertEqual(admin_emails(self.club), ["admin@ajax.example"])
def make_overdue_club(self, name, *, auto_archive):
# A separate club rather than resubscribing self.club: open_period() caches
# club.subscription on the instance it's given, so calling subscribe() twice
# against the SAME Python object -- only possible by reusing one across two calls,
# never a real request -- would read the first call's now-stale cached subscription.
club = Club.objects.create(name=name)
user = User.objects.create_user(email=f"{name.lower()}@ajax.example", password="pw-secret-123")
member = Member.objects.create(user=user, first_name="A", last_name="Admin")
ClubRole.objects.create(club=club, member=member, role=ClubRole.Roles.ADMIN)
subscribe(club, self.plan, start=self.today - datetime.timedelta(days=DEFAULT_GRACE_DAYS + 5), auto_archive=auto_archive)
return club
def test_an_overdue_reminder_threatens_archiving_when_it_will_happen(self):
club = self.make_overdue_club("Archive On", auto_archive=True)
notice = club_billing_notice(club, self.today)
self.assertTrue(notice.will_archive)
send_reminder(club, notice, recipients=["archive-on@ajax.example"])
self.assertIn("about to be archived", mail.outbox[0].subject)
self.assertIn("archived", mail.outbox[0].body)
def test_an_overdue_reminder_does_not_threaten_archiving_when_auto_archive_is_off(self):
# This is the bug the two-column-modal review turned up: the subject branched only
# on notice.level, so a club that will NEVER be archived still got told it was
# "about to be archived" -- while the body correctly said otherwise.
club = self.make_overdue_club("Archive Off", auto_archive=False)
notice = club_billing_notice(club, self.today)
self.assertFalse(notice.will_archive)
send_reminder(club, notice, recipients=["archive-off@ajax.example"])
self.assertNotIn("about to be archived", mail.outbox[0].subject)
self.assertNotIn("archived", mail.outbox[0].body)
self.assertIn("good standing", mail.outbox[0].body)
def test_sending_records_the_level_and_fills_the_outbox(self):
notice = club_billing_notice(self.club, self.today)
send_reminder(self.club, notice, recipients=["admin@ajax.example"])

5
controlpanel/forms.py
View File

@@ -140,6 +140,11 @@ class TrialForm(forms.Form):
trial_plan = forms.ModelChoiceField(queryset=Plan.objects.none(), label=_("Trial plan"), help_text=_("What this club is billed on during the trial. Its length is the plan's own duration."))
post_trial_plan = forms.ModelChoiceField(queryset=Plan.objects.none(), label=_("Then switch to"), help_text=_("The plan it lands on automatically once the trial ends."))
start = forms.DateField(required=False, widget=forms.DateInput(attrs={"type": "date"}), label=_("Trial starts"), help_text=_("Left blank, the trial starts today."))
# Same two switches SubscriptionForm offers. Without them here a trial could only be
# started on the defaults, and the only way to change them afterwards is the "Change
# plan" modal -- which ends the trial as a side effect.
auto_renew = forms.BooleanField(required=False, initial=True, label=_("Auto renew"), help_text=_("Issue the next period automatically before this one ends."))
auto_archive = forms.BooleanField(required=False, initial=True, label=_("Auto archive"), help_text=_("Archive this club when a period goes unpaid past its grace period."))
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

17
controlpanel/tests.py
View File

@@ -1502,7 +1502,10 @@ class TrialPanelTests(ControlPanelTestBase):
PlanPrice.objects.create(plan=self.plan, active_from=self.today - datetime.timedelta(days=1200), amount=Decimal("500.00"))
def start_trial(self, **data):
data = {"trial_plan": self.trial_plan.pk, "post_trial_plan": self.plan.pk, "start": ""} | data
# "on" for both, matching how a freshly opened modal actually renders: unbound,
# TrialForm's auto_renew/auto_archive are initial=True, so a real submission that
# never touches either checkbox posts them checked.
data = {"trial_plan": self.trial_plan.pk, "post_trial_plan": self.plan.pk, "start": "", "auto_renew": "on", "auto_archive": "on"} | data
return self.client.post(reverse("controlpanel:club_trial_start", args=[self.club.pk]), data)
def test_starting_a_trial_opens_a_short_first_period(self):
@@ -1541,6 +1544,18 @@ class TrialPanelTests(ControlPanelTestBase):
self.assertContains(response, "On trial")
self.assertContains(response, "Standard")
def test_a_trial_can_be_started_with_auto_archive_off(self):
# Same switch SubscriptionForm offers -- a trial must be able to opt out of
# auto-archiving too, not just a paid subscription.
self.start_trial(auto_archive="")
self.assertFalse(self.club.subscription.auto_archive)
def test_a_trial_defaults_to_auto_archive_on(self):
self.start_trial()
self.assertTrue(self.club.subscription.auto_archive)
def test_manually_changing_plan_mid_trial_clears_the_trial(self):
self.start_trial()
other = Plan.objects.create(name="Other")

2
controlpanel/views.py
View File

@@ -550,6 +550,8 @@ class ClubStartTrialView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, For
trial_plan,
post_trial_plan=form.cleaned_data["post_trial_plan"],
start=form.cleaned_data.get("start"),
auto_renew=form.cleaned_data["auto_renew"],
auto_archive=form.cleaned_data["auto_archive"],
)
notify(self.request, f"s|Trial started|{club} is on a {trial_plan.duration_months}-month trial of {trial_plan}, then switches to {form.cleaned_data['post_trial_plan']}.")