Merge pull request #969 from ae-utbm/default-groups

Give default groups to users

club/tests.py

@@ -213,9 +213,9 @@ class TestMembershipQuerySet(TestClub):
             memberships[1].club.members_group,
             memberships[1].club.board_group,
         }
-        assert set(user.groups.all()) == club_groups
+        assert set(user.groups.all()).issuperset(club_groups)
         user.memberships.all().delete()
-        assert user.groups.all().count() == 0
+        assert set(user.groups.all()).isdisjoint(club_groups)
 
 
 class TestClubModel(TestClub):

core/management/commands/populate_more.py

@@ -5,6 +5,7 @@ from typing import Iterator
 
 from dateutil.relativedelta import relativedelta
 from django.conf import settings
+from django.contrib.auth.hashers import make_password
 from django.core.management.base import BaseCommand
 from django.db.models import Count, Exists, Min, OuterRef, Subquery
 from django.utils.timezone import localdate, make_aware, now
@@ -38,26 +39,10 @@ class Command(BaseCommand):
             raise Exception("Never call this command in prod. Never.")
 
         self.stdout.write("Creating users...")
-        users = [
+        users = self.create_users()
-            User(
-                username=self.faker.user_name(),
-                first_name=self.faker.first_name(),
-                last_name=self.faker.last_name(),
-                date_of_birth=self.faker.date_of_birth(minimum_age=15, maximum_age=25),
-                email=self.faker.email(),
-                phone=self.faker.phone_number(),
-                address=self.faker.address(),
-            )
-            for _ in range(600)
-        ]
-        # there may a duplicate or two
-        # Not a problem, we will just have 599 users instead of 600
-        User.objects.bulk_create(users, ignore_conflicts=True)
-        users = list(User.objects.order_by("-id")[: len(users)])
-
         subscribers = random.sample(users, k=int(0.8 * len(users)))
         self.stdout.write("Creating subscriptions...")
-        self.create_subscriptions(users)
+        self.create_subscriptions(subscribers)
         self.stdout.write("Creating club memberships...")
         users_qs = User.objects.filter(id__in=[s.id for s in subscribers])
         subscribers_now = list(
@@ -102,11 +87,34 @@ class Command(BaseCommand):
 
         self.stdout.write("Done")
 
+    def create_users(self) -> list[User]:
+        password = make_password("plop")
+        users = [
+            User(
+                username=self.faker.user_name(),
+                first_name=self.faker.first_name(),
+                last_name=self.faker.last_name(),
+                date_of_birth=self.faker.date_of_birth(minimum_age=15, maximum_age=25),
+                email=self.faker.email(),
+                phone=self.faker.phone_number(),
+                address=self.faker.address(),
+                password=password,
+            )
+            for _ in range(600)
+        ]
+        # there may a duplicate or two
+        # Not a problem, we will just have 599 users instead of 600
+        users = User.objects.bulk_create(users, ignore_conflicts=True)
+        users = list(User.objects.order_by("-id")[: len(users)])
+        public_group = Group.objects.get(pk=settings.SITH_GROUP_PUBLIC_ID)
+        public_group.users.add(*users)
+        return users
+
     def create_subscriptions(self, users: list[User]):
-        def prepare_subscription(user: User, start_date: date) -> Subscription:
+        def prepare_subscription(_user: User, start_date: date) -> Subscription:
             payment_method = random.choice(settings.SITH_SUBSCRIPTION_PAYMENT_METHOD)[0]
             duration = random.randint(1, 4)
-            sub = Subscription(member=user, payment_method=payment_method)
+            sub = Subscription(member=_user, payment_method=payment_method)
             sub.subscription_start = sub.compute_start(d=start_date, duration=duration)
             sub.subscription_end = sub.compute_end(duration)
             return sub
@@ -130,6 +138,10 @@ class Command(BaseCommand):
                     user, self.faker.past_date(sub.subscription_end)
                 )
                 subscriptions.append(sub)
+        old_subscriber_group = Group.objects.get(
+            pk=settings.SITH_GROUP_OLD_SUBSCRIBERS_ID
+        )
+        old_subscriber_group.users.add(*users)
         Subscription.objects.bulk_create(subscriptions)
         Customer.objects.bulk_create(customers, ignore_conflicts=True)
 

core/models.py

@@ -320,12 +320,16 @@ class User(AbstractUser):
         return self.get_display_name()
 
     def save(self, *args, **kwargs):
+        adding = self._state.adding
         with transaction.atomic():
-            if self.id:
+            if not adding:
                 old = User.objects.filter(id=self.id).first()
                 if old and old.username != self.username:
                     self._change_username(self.username)
             super().save(*args, **kwargs)
+            if adding:
+                # All users are in the public group.
+                self.groups.add(settings.SITH_GROUP_PUBLIC_ID)
 
     def get_absolute_url(self) -> str:
         return reverse("core:user_profile", kwargs={"user_id": self.pk})
@@ -380,12 +384,8 @@ class User(AbstractUser):
             raise ValueError("You must either provide the id or the name of the group")
         if group is None:
             return False
-        if group.id == settings.SITH_GROUP_PUBLIC_ID:
-            return True
         if group.id == settings.SITH_GROUP_SUBSCRIBERS_ID:
             return self.is_subscribed
-        if group.id == settings.SITH_GROUP_OLD_SUBSCRIBERS_ID:
-            return self.was_subscribed
         if group.id == settings.SITH_GROUP_ROOT_ID:
             return self.is_root
         return group in self.cached_groups

core/tests/test_user.py

@@ -187,3 +187,11 @@ def test_generate_username(first_name: str, last_name: str, expected: str):
     new_user = User(first_name=first_name, last_name=last_name, email="a@example.com")
     new_user.generate_username()
     assert new_user.username == expected
+
+
+@pytest.mark.django_db
+def test_user_added_to_public_group():
+    """Test that newly created users are added to the public group"""
+    user = baker.make(User)
+    assert user.groups.filter(pk=settings.SITH_GROUP_PUBLIC_ID).exists()
+    assert user.is_in_group(pk=settings.SITH_GROUP_PUBLIC_ID)

rootplace/tests/test_merge_users.py

@@ -14,6 +14,7 @@
 #
 from datetime import timedelta
 
+from django.conf import settings
 from django.test import TestCase
 from django.urls import reverse
 from django.utils.timezone import localtime, now
@@ -71,10 +72,12 @@ class TestMergeUser(TestCase):
         assert self.to_keep.nick_name == "B'ian"
         assert self.to_keep.address == "Jerusalem"
         assert self.to_keep.parent_address == "Rome"
-        assert self.to_keep.groups.count() == 3
+        assert set(self.to_keep.groups.values_list("id", flat=True)) == {
-        groups = sorted(self.to_keep.groups.all(), key=lambda i: i.id)
+            settings.SITH_GROUP_PUBLIC_ID,
-        expected = sorted([subscribers, mde_admin, sas_admin], key=lambda i: i.id)
+            subscribers.id,
-        assert groups == expected
+            mde_admin.id,
+            sas_admin.id,
+        }
 
     def test_both_subscribers_and_with_account(self):
         Customer(user=self.to_keep, account_id="11000l", amount=0).save()

subscription/models.py

@@ -76,8 +76,11 @@ class Subscription(models.Model):
         super().save()
         from counter.models import Customer
 
-        _, created = Customer.get_or_create(self.member)
+        _, account_created = Customer.get_or_create(self.member)
-        if created:
+        if account_created:
+            # 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(