From 2086d23b5065fe9d13ae0bb9910c5bc984068c70 Mon Sep 17 00:00:00 2001
From: imperosol <thgirod@hotmail.com>
Date: Sun, 31 Aug 2025 20:37:44 +0200
Subject: [PATCH] fix old subscribers group attribution
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Si un utilisateur faisait sa première cotisation alors qu'il avait déjà un compte AE (par exemple, en effectuant un achat sur l'eboutic avant sa cotisation), alors il pouvait se retrouver hors du groupe Anciens cotisants.
---
 subscription/models.py                        | 21 +++++++------------
 ...sbcription.py => test_new_subscription.py} | 19 ++++++++++++++++-
 subscription/views.py                         | 15 ++++++++++++-
 3 files changed, 40 insertions(+), 15 deletions(-)
 rename subscription/tests/{test_new_susbcription.py => test_new_subscription.py} (89%)

diff --git a/subscription/models.py b/subscription/models.py
index 38d6c0f5..1d182ad0 100644
--- a/subscription/models.py
+++ b/subscription/models.py
@@ -18,7 +18,6 @@ from datetime import date, timedelta
 
 from dateutil.relativedelta import relativedelta
 from django.conf import settings
-from django.contrib.auth.forms import PasswordResetForm
 from django.core.exceptions import ValidationError
 from django.db import models
 from django.urls import reverse
@@ -73,23 +72,19 @@ class Subscription(models.Model):
             return f"No user - {self.pk}"
 
     def save(self, *args, **kwargs):
-        super().save()
         from counter.models import Customer
 
-        _, account_created = Customer.get_or_create(self.member)
-        if account_created:
+        if not self.member.was_subscribed:
+            # This is the first ever subscription for this user
+            customer, _ = Customer.get_or_create(self.member)
             # Someone who subscribed once will be considered forever
             # as an old subscriber.
             self.member.groups.add(settings.SITH_GROUP_OLD_SUBSCRIBERS_ID)
-            form = PasswordResetForm({"email": self.member.email})
-            if form.is_valid():
-                form.save(
-                    use_https=True,
-                    email_template_name="core/new_user_email.jinja",
-                    subject_template_name="core/new_user_email_subject.jinja",
-                    from_email="ae@utbm.fr",
-                )
-        self.member.make_home()
+            self.member.make_home()
+            # now that the user is an old subscriber, change the cached
+            # property accordingly
+            self.member.__dict__["was_subscribed"] = True
+        super().save()
 
     def get_absolute_url(self):
         return reverse("core:user_edit", kwargs={"user_id": self.member_id})
diff --git a/subscription/tests/test_new_susbcription.py b/subscription/tests/test_new_subscription.py
similarity index 89%
rename from subscription/tests/test_new_susbcription.py
rename to subscription/tests/test_new_subscription.py
index fd23d9dd..1c4eaa6b 100644
--- a/subscription/tests/test_new_susbcription.py
+++ b/subscription/tests/test_new_subscription.py
@@ -5,6 +5,7 @@ from typing import Callable
 
 import pytest
 from dateutil.relativedelta import relativedelta
+from django.conf import settings
 from django.contrib.auth.models import Permission
 from django.test import Client
 from django.urls import reverse
@@ -14,8 +15,10 @@ from pytest_django.asserts import assertRedirects
 from pytest_django.fixtures import SettingsWrapper
 
 from core.baker_recipes import board_user, old_subscriber_user, subscriber_user
-from core.models import User
+from core.models import Group, User
+from counter.models import Customer
 from subscription.forms import SubscriptionExistingUserForm, SubscriptionNewUserForm
+from subscription.models import Subscription
 
 
 @pytest.mark.django_db
@@ -189,3 +192,17 @@ def test_submit_form_new_user(client: Client, settings: SettingsWrapper):
             kwargs={"subscription_id": current_subscription.id},
         ),
     )
+
+
+@pytest.mark.django_db
+def test_subscription_for_user_that_had_a_sith_account():
+    """Test that a newly subscribed user is added to the old subscribers group,
+    even if there already was a sith account (e.g. created during an eboutic purchase).
+    """
+    user = baker.make(User)
+    Customer.get_or_create(user)
+    group = Group.objects.get(id=settings.SITH_GROUP_OLD_SUBSCRIBERS_ID)
+    assert not user.groups.contains(group)
+    subscription = baker.prepare(Subscription, member=user)
+    subscription.save()
+    assert user.groups.contains(group)
diff --git a/subscription/views.py b/subscription/views.py
index d5f9d75d..035f9035 100644
--- a/subscription/views.py
+++ b/subscription/views.py
@@ -14,6 +14,7 @@
 #
 
 from django.conf import settings
+from django.contrib.auth.forms import PasswordResetForm
 from django.contrib.auth.mixins import PermissionRequiredMixin
 from django.core.exceptions import PermissionDenied
 from django.urls import reverse, reverse_lazy
@@ -65,11 +66,23 @@ class CreateSubscriptionExistingUserFragment(CreateSubscriptionFragment):
 
 
 class CreateSubscriptionNewUserFragment(CreateSubscriptionFragment):
-    """Create a subscription for a user who already exists."""
+    """Create a subscription for a user who doesn't exist yet."""
 
     form_class = SubscriptionNewUserForm
     extra_context = {"post_url": reverse_lazy("subscription:fragment-new-user")}
 
+    def form_valid(self, form):
+        res = super().form_valid(form)
+        reset_form = PasswordResetForm({"email": form.cleaned_data["email"]})
+        if reset_form.is_valid():
+            reset_form.save(
+                use_https=True,
+                email_template_name="core/new_user_email.jinja",
+                subject_template_name="core/new_user_email_subject.jinja",
+                from_email="ae@utbm.fr",
+            )
+        return res
+
 
 class SubscriptionCreatedFragment(PermissionRequiredMixin, DetailView):
     template_name = "subscription/fragments/creation_success.jinja"