Mise à jour d'avril (#643)

.github/actions/compile_messages/action.yml

@@ -0,0 +1,8 @@
+name: "Compile messages"
+description: "Compile the gettext translation messages"
+runs:
+  using: composite
+  steps:
+      - name: Setup project
+        run: poetry run ./manage.py compilemessages
+        shell: bash

.github/actions/setup_project/action.yml

@@ -0,0 +1,53 @@
+name: "Setup project"
+description: "Setup Python and Poetry"
+runs:
+  using: composite
+  steps:
+    - name: Install apt packages
+      uses: awalsh128/cache-apt-pkgs-action@latest
+      with:
+        packages: gettext libxapian-dev libgraphviz-dev
+        version: 1.0  # increment to reset cache
+
+    - name: Install dependencies
+      run: |
+        sudo apt update
+        sudo apt install gettext libxapian-dev libgraphviz-dev
+      shell: bash
+
+    - name: Set up python
+      uses: actions/setup-python@v4
+      with:
+        python-version: "3.10"
+
+    - name: Load cached Poetry installation
+      id: cached-poetry
+      uses: actions/cache@v3
+      with:
+        path: ~/.local
+        key: poetry-0  # increment to reset cache
+
+    - name: Install Poetry
+      if: steps.cached-poetry.outputs.cache-hit != 'true'
+      shell: bash
+      run: curl -sSL https://install.python-poetry.org | python3 -
+
+    - name: Check pyproject.toml syntax
+      shell: bash
+      run: poetry check
+
+    - name: Load cached dependencies
+      uses: actions/cache@v3
+      with:
+        path: ~/.cache/pypoetry
+        key: ${{ runner.os }}-poetry-${{ hashFiles('**/poetry.lock') }}
+        restore-keys: |
+          ${{ runner.os }}-poetry-
+
+    - name: Install dependencies
+      run: poetry install -E testing -E docs
+      shell: bash
+
+    - name: Compile gettext messages
+      run: poetry run ./manage.py compilemessages
+      shell: bash

.github/actions/setup_xapian/action.yml

@@ -0,0 +1,10 @@
+name: "Setup xapian"
+description: "Setup the xapian indexes"
+runs:
+  using: composite
+  steps:
+    - name: Setup xapian index
+      run: |
+        mkdir -p /dev/shm/search_indexes
+        ln -s /dev/shm/search_indexes sith/search_indexes
+      shell: bash

.github/workflows/ci.yml

@@ -0,0 +1,43 @@
+name: Sith 3 CI
+
+on:
+  push:
+    branches:
+      - master
+      - taiste
+  pull_request:
+    branches:
+      - master
+      - taiste
+
+jobs:
+  black:
+    name: Black format
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out repository
+        uses: actions/checkout@v3
+      - name: Setup Project
+        uses: ./.github/actions/setup_project
+      - run: poetry run black --check .
+
+  tests:
+    name: Run tests and generate coverage report
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out repository
+        uses: actions/checkout@v3
+      - uses: ./.github/actions/setup_project
+      - uses: ./.github/actions/setup_xapian
+      - uses: ./.github/actions/compile_messages
+      - name: Run tests
+        run: poetry run coverage run ./manage.py test
+      - name: Generate coverage report
+        run: |
+          poetry run coverage report
+          poetry run coverage html
+      - name: Archive code coverage results
+        uses: actions/upload-artifact@v3
+        with:
+          name: coverage-report
+          path: coverage_report

.github/workflows/unittests.yml

@@ -1,83 +0,0 @@
-name: Sith3 CI
-
-on:
-  pull_request:
-    branches: [ master, taiste ]
-  push:
-    branches: [ master, taiste ]
-    
-jobs:
-  unittests:
-    runs-on: ubuntu-latest
-    timeout-minutes: 30
-
-    steps:
-      - uses: actions/checkout@v3
-
-      # Skip unit testing if no diff on .py files
-      - name: Check file diff
-        uses: technote-space/get-diff-action@v6
-        id: git-diff
-        with:
-          PATTERNS: |
-            **/*.py
-
-      - name: Set up python
-        if: steps.git-diff.outputs.diff
-        uses: actions/setup-python@v4
-        with:
-          python-version: '3.8'
-
-      - name: Install dependencies
-        if: steps.git-diff.outputs.diff
-        run: |
-          sudo apt-get update
-          sudo apt-get install gettext libxapian-dev libgraphviz-dev
-
-      - name: Install poetry
-        if: steps.git-diff.outputs.diff
-        run: |
-          python -m pip install --upgrade pip
-          python -m pip install poetry
-
-      - name: Checking pyproject.toml syntax
-        if: steps.git-diff.outputs.diff
-        run: poetry check
-
-      - name: Install project
-        if: steps.git-diff.outputs.diff
-        run: poetry install -E testing
-
-      - name: Setup xapian index
-        if: steps.git-diff.outputs.diff
-        run: |
-          mkdir -p /dev/shm/search_indexes
-          ln -s /dev/shm/search_indexes sith/search_indexes
-
-      - name: Setup project
-        if: steps.git-diff.outputs.diff
-        run: poetry run ./manage.py compilemessages
-      
-      - name: Launch tests and generate coverage report
-        if: steps.git-diff.outputs.diff
-        run: |
-          poetry run coverage run ./manage.py test
-          poetry run coverage report
-  
-  lint:
-    runs-on: ubuntu-latest
-
-    steps:
-      - uses: actions/checkout@v3
-      - name: Set up python
-        uses: actions/setup-python@v4
-        with:
-          python-version: '3.8'
-
-      - name: Install black
-        run: |
-          python -m pip install --upgrade pip
-          python -m pip install black==22.6.0
-
-      - name: Check linting
-        run: black --check .

accounting/migrations/0001_initial.py

@@ -8,7 +8,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = []
 
     operations = [

accounting/migrations/0002_auto_20160824_2152.py

@@ -6,7 +6,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [
         ("club", "0001_initial"),
         ("accounting", "0001_initial"),

accounting/migrations/0003_auto_20160824_2203.py

@@ -6,7 +6,6 @@ import phonenumber_field.modelfields
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("accounting", "0002_auto_20160824_2152")]
 
     operations = [

accounting/migrations/0004_auto_20161005_1505.py

@@ -6,7 +6,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("accounting", "0003_auto_20160824_2203")]
 
     operations = [

accounting/migrations/0005_auto_20170324_0917.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("accounting", "0004_auto_20161005_1505")]
 
     operations = [

accounting/models.py

@@ -66,7 +66,7 @@ class Company(models.Model):
         """
         Method to see if that object can be edited by the given user
         """
-        if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID):
+        if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID):
             return True
         return False
 
@@ -117,7 +117,9 @@ class BankAccount(models.Model):
         """
         Method to see if that object can be edited by the given user
         """
-        if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID):
+        if user.is_anonymous:
+            return False
+        if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID):
             return True
         m = self.club.get_membership_for(user)
         if m is not None and m.role >= settings.SITH_CLUB_ROLES_ID["Treasurer"]:
@@ -154,7 +156,9 @@ class ClubAccount(models.Model):
         """
         Method to see if that object can be edited by the given user
         """
-        if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID):
+        if user.is_anonymous:
+            return False
+        if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID):
             return True
         return False
 
@@ -225,7 +229,9 @@ class GeneralJournal(models.Model):
         """
         Method to see if that object can be edited by the given user
         """
-        if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID):
+        if user.is_anonymous:
+            return False
+        if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID):
             return True
         if self.club_account.can_be_edited_by(user):
             return True
@@ -235,7 +241,7 @@ class GeneralJournal(models.Model):
         """
         Method to see if that object can be edited by the given user
         """
-        if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID):
+        if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID):
             return True
         if self.club_account.can_be_edited_by(user):
             return True
@@ -414,7 +420,9 @@ class Operation(models.Model):
         """
         Method to see if that object can be edited by the given user
         """
-        if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID):
+        if user.is_anonymous:
+            return False
+        if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID):
             return True
         if self.journal.closed:
             return False
@@ -427,7 +435,7 @@ class Operation(models.Model):
         """
         Method to see if that object can be edited by the given user
         """
-        if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID):
+        if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID):
             return True
         if self.journal.closed:
             return False
@@ -483,7 +491,9 @@ class AccountingType(models.Model):
         """
         Method to see if that object can be edited by the given user
         """
-        if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID):
+        if user.is_anonymous:
+            return False
+        if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID):
             return True
         return False
 
@@ -554,6 +564,8 @@ class Label(models.Model):
         )
 
     def is_owned_by(self, user):
+        if user.is_anonymous:
+            return False
         return self.club_account.is_owned_by(user)
 
     def can_be_edited_by(self, user):

accounting/templates/accounting/bank_account_details.jinja

@@ -12,7 +12,7 @@
         </p>
         <hr>
         <h2>{% trans %}Bank account: {% endtrans %}{{ object.name }}</h2>
-        {% if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) and not object.club_accounts.exists() %}
+        {% if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) and not object.club_accounts.exists() %}
         <a href="{{ url('accounting:bank_delete', b_account_id=object.id) }}">{% trans %}Delete{% endtrans %}</a>
         {% endif %}
         <h4>{% trans %}Infos{% endtrans %}</h4>

accounting/templates/accounting/bank_account_list.jinja

@@ -9,7 +9,7 @@
         <h4>
         {% trans %}Accounting{% endtrans %}
         </h4>
-        {% if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %}
+        {% if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %}
         <p><a href="{{ url('accounting:simple_type_list') }}">{% trans %}Manage simplified types{% endtrans %}</a></p>
         <p><a href="{{ url('accounting:type_list') }}">{% trans %}Manage accounting types{% endtrans %}</a></p>
         <p><a href="{{ url('accounting:bank_new') }}">{% trans %}New bank account{% endtrans %}</a></p>

accounting/templates/accounting/club_account_details.jinja

@@ -16,7 +16,7 @@
         {% if user.is_root and not object.journals.exists() %}
         <a href="{{ url('accounting:club_delete', c_account_id=object.id) }}">{% trans %}Delete{% endtrans %}</a>
         {% endif %}
-        {% if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %}
+        {% if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %}
         <p><a href="{{ url('accounting:label_new') }}?parent={{ object.id }}">{% trans %}New label{% endtrans %}</a></p>
         {% endif %}
         <p><a href="{{ url('accounting:label_list', clubaccount_id=object.id) }}">{% trans %}Label list{% endtrans %}</a></p>
@@ -56,7 +56,7 @@
                 {% endif %}
                 <td> <a href="{{ url('accounting:journal_details', j_id=j.id) }}">{% trans %}View{% endtrans %}</a>
                     <a href="{{ url('accounting:journal_edit', j_id=j.id) }}">{% trans %}Edit{% endtrans %}</a>
-                    {% if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) and j.operations.count() == 0 %}
+                    {% if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) and j.operations.count() == 0 %}
                         <a href="{{ url('accounting:journal_delete', j_id=j.id) }}">{% trans %}Delete{% endtrans %}</a>
                     {% endif %}
                     </td>

accounting/templates/accounting/co_list.jinja

@@ -6,11 +6,12 @@
 
 {% block content %}
     <div id="accounting">
-        {% if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) or user.is_root %}
+        {% if user.is_root
+           or user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID)
+        %}
         <p><a href="{{ url('accounting:co_new') }}">{% trans %}Create new company{% endtrans %}</a></p>
         {% endif %}
-
-        </br>
+        <br/>
         <table>
             <thead>
             <tr>

accounting/templates/accounting/journal_details.jinja

@@ -84,10 +84,13 @@
                 <td>-</td>
                 {% endif %}
                 <td>
-                    {% if o.journal.club_account.bank_account.name != "AE TI" and o.journal.club_account.bank_account.name != "TI" or user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %}
-                    {% if not o.journal.closed %}
-                    <a href="{{ url('accounting:op_edit', op_id=o.id) }}">{% trans %}Edit{% endtrans %}</a>
-                    {% endif %}
+                    {%
+                        if o.journal.club_account.bank_account.name not in ["AE TI", "TI"]
+                        or user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID)
+                    %}
+                        {% if not o.journal.closed %}
+                            <a href="{{ url('accounting:op_edit', op_id=o.id) }}">{% trans %}Edit{% endtrans %}</a>
+                        {% endif %}
                     {% endif %}
                 </td>
                 <td><a href="{{ url('accounting:op_pdf', op_id=o.id) }}">{% trans %}Generate{% endtrans %}</a></td>

accounting/templates/accounting/label_list.jinja

@@ -13,7 +13,7 @@
         </p>
         <hr>
         <p><a href="{{ url('accounting:club_details', c_account_id=object.id) }}">{% trans %}Back to club account{% endtrans %}</a></p>
-        {% if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %}
+        {% if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %}
         <p><a href="{{ url('accounting:label_new') }}?parent={{ object.id }}">{% trans %}New label{% endtrans %}</a></p>
         {% endif %}
         {% if object.labels.all() %}
@@ -21,7 +21,7 @@
         <ul>
             {% for l in object.labels.all()  %}
             <li><a href="{{ url('accounting:label_edit', label_id=l.id) }}">{{ l }}</a>
-            {% if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %}
+            {% if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %}
              -
                 <a href="{{ url('accounting:label_delete', label_id=l.id) }}">{% trans %}Delete{% endtrans %}</a>
             {% endif %}

accounting/tests.py

@@ -31,7 +31,6 @@ from accounting.models import (
 
 class RefoundAccountTest(TestCase):
     def setUp(self):
-        call_command("populate")
         self.skia = User.objects.filter(username="skia").first()
         # reffil skia's account
         self.skia.customer.amount = 800
@@ -73,7 +72,6 @@ class RefoundAccountTest(TestCase):
 
 class JournalTest(TestCase):
     def setUp(self):
-        call_command("populate")
         self.journal = GeneralJournal.objects.filter(id=1).first()
 
     def test_permission_granted(self):
@@ -101,7 +99,6 @@ class JournalTest(TestCase):
 
 class OperationTest(TestCase):
     def setUp(self):
-        call_command("populate")
         self.tomorrow_formatted = (date.today() + timedelta(days=1)).strftime(
             "%d/%m/%Y"
         )

accounting/views.py

@@ -891,7 +891,7 @@ class RefoundAccountView(FormView):
     form_class = CloseCustomerAccountForm
 
     def permission(self, user):
-        if user.is_root or user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID):
+        if user.is_root or user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID):
             return True
         else:
             raise PermissionDenied

api/views/counter.py

@@ -24,7 +24,6 @@ from api.views import RightModelViewSet
 
 
 class CounterSerializer(serializers.ModelSerializer):
-
     is_open = serializers.BooleanField(read_only=True)
     barman_list = serializers.ListField(
         child=serializers.IntegerField(), read_only=True

api/views/launderette.py

@@ -24,7 +24,6 @@ from api.views import RightModelViewSet
 
 
 class LaunderettePlaceSerializer(serializers.ModelSerializer):
-
     machine_list = serializers.ListField(
         child=serializers.IntegerField(), read_only=True
     )

club/forms.py

@@ -167,7 +167,6 @@ class SellingsForm(forms.Form):
     )
 
     def __init__(self, club, *args, **kwargs):
-
         super(SellingsForm, self).__init__(*args, **kwargs)
         self.fields["products"] = forms.ModelMultipleChoiceField(
             club.products.order_by("name").filter(archived=False).all(),
@@ -230,9 +229,7 @@ class ClubMemberForm(forms.Form):
                 id__in=[
                     ms.user.id
                     for ms in self.club_members
-                    if ms.can_be_edited_by(
-                        self.request_user, self.request_user_membership
-                    )
+                    if ms.can_be_edited_by(self.request_user)
                 ]
             ).all(),
             label=_("Mark as old"),

club/migrations/0001_initial.py

@@ -7,7 +7,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = []
 
     operations = [

club/migrations/0002_auto_20160824_2152.py

@@ -7,7 +7,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [
         migrations.swappable_dependency(settings.AUTH_USER_MODEL),
         ("club", "0001_initial"),

club/migrations/0003_auto_20160902_2042.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("club", "0002_auto_20160824_2152")]
 
     operations = [

club/migrations/0004_auto_20160915_1057.py

@@ -7,7 +7,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("club", "0003_auto_20160902_2042")]
 
     operations = [

club/migrations/0005_auto_20161120_1149.py

@@ -6,7 +6,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("club", "0004_auto_20160915_1057")]
 
     operations = [

club/migrations/0006_auto_20161229_0040.py

@@ -6,7 +6,6 @@ import django.utils.timezone
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("club", "0005_auto_20161120_1149")]
 
     operations = [

club/migrations/0007_auto_20170324_0917.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("club", "0006_auto_20161229_0040")]
 
     operations = [

club/migrations/0008_auto_20170515_2214.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("club", "0007_auto_20170324_0917")]
 
     operations = [

club/migrations/0009_auto_20170822_2232.py

@@ -9,7 +9,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [
         migrations.swappable_dependency(settings.AUTH_USER_MODEL),
         ("club", "0008_auto_20170515_2214"),

club/migrations/0010_auto_20170912_2028.py

@@ -19,7 +19,6 @@ def generate_club_pages(apps, schema_editor):
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0024_auto_20170906_1317"), ("club", "0010_club_logo")]
 
     operations = [

club/migrations/0010_club_logo.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("club", "0009_auto_20170822_2232")]
 
     operations = [

club/migrations/0011_auto_20180426_2013.py

@@ -7,7 +7,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("club", "0010_auto_20170912_2028")]
 
     operations = [

club/models.py

@@ -22,10 +22,14 @@
 # Place - Suite 330, Boston, MA 02111-1307, USA.
 #
 #
+from typing import Optional
 
+from django.core.cache import cache
 from django.db import models
 from django.core import validators
 from django.conf import settings
+from django.db.models import Q
+from django.utils.timezone import now
 from django.utils.translation import gettext_lazy as _
 from django.core.exceptions import ValidationError, ObjectDoesNotExist
 from django.db import transaction
@@ -72,6 +76,7 @@ class Club(models.Model):
         _("short description"), max_length=1000, default="", blank=True, null=True
     )
     address = models.CharField(_("address"), max_length=254)
+
     # This function prevents generating migration upon settings change
     def get_default_owner_group():
         return settings.SITH_GROUP_ROOT_ID
@@ -122,12 +127,22 @@ class Club(models.Model):
     def clean(self):
         self.check_loop()
 
-    def _change_unixname(self, new_name):
+    def _change_unixname(self, old_name, new_name):
         c = Club.objects.filter(unix_name=new_name).first()
         if c is None:
+            # Update all the groups names
+            Group.objects.filter(name=old_name).update(name=new_name)
+            Group.objects.filter(name=old_name + settings.SITH_BOARD_SUFFIX).update(
+                name=new_name + settings.SITH_BOARD_SUFFIX
+            )
+            Group.objects.filter(name=old_name + settings.SITH_MEMBER_SUFFIX).update(
+                name=new_name + settings.SITH_MEMBER_SUFFIX
+            )
+
             if self.home:
                 self.home.name = new_name
                 self.home.save()
+
         else:
             raise ValidationError(_("A club with that unix_name already exists"))
 
@@ -171,29 +186,34 @@ class Club(models.Model):
             self.page.parent = self.parent.page
             self.page.save(force_lock=True)
 
+    @transaction.atomic()
     def save(self, *args, **kwargs):
-        with transaction.atomic():
-            creation = False
-            old = Club.objects.filter(id=self.id).first()
-            if not old:
-                creation = True
-            else:
-                if old.unix_name != self.unix_name:
-                    self._change_unixname(self.unix_name)
-            super(Club, self).save(*args, **kwargs)
-            if creation:
-                board = MetaGroup(name=self.unix_name + settings.SITH_BOARD_SUFFIX)
-                board.save()
-                member = MetaGroup(name=self.unix_name + settings.SITH_MEMBER_SUFFIX)
-                member.save()
-                subscribers = Group.objects.filter(
-                    name=settings.SITH_MAIN_MEMBERS_GROUP
-                ).first()
-                self.make_home()
-                self.home.edit_groups.set([board])
-                self.home.view_groups.set([member, subscribers])
-                self.home.save()
-            self.make_page()
+        old = Club.objects.filter(id=self.id).first()
+        creation = old is None
+        if not creation and old.unix_name != self.unix_name:
+            self._change_unixname(self.unix_name)
+        super(Club, self).save(*args, **kwargs)
+        if creation:
+            board = MetaGroup(name=self.unix_name + settings.SITH_BOARD_SUFFIX)
+            board.save()
+            member = MetaGroup(name=self.unix_name + settings.SITH_MEMBER_SUFFIX)
+            member.save()
+            subscribers = Group.objects.filter(
+                name=settings.SITH_MAIN_MEMBERS_GROUP
+            ).first()
+            self.make_home()
+            self.home.edit_groups.set([board])
+            self.home.view_groups.set([member, subscribers])
+            self.home.save()
+        self.make_page()
+        cache.set(f"sith_club_{self.unix_name}", self)
+
+    def delete(self, *args, **kwargs):
+        super().delete(*args, **kwargs)
+        # Invalidate the cache of this club and of its memberships
+        for membership in self.members.ongoing().select_related("user"):
+            cache.delete(f"membership_{self.id}_{membership.user.id}")
+        cache.delete(f"sith_club_{self.unix_name}")
 
     def __str__(self):
         return self.name
@@ -208,7 +228,9 @@ class Club(models.Model):
         """
         Method to see if that object can be super edited by the given user
         """
-        return user.is_in_group(settings.SITH_MAIN_BOARD_GROUP)
+        if user.is_anonymous:
+            return False
+        return user.is_board_member
 
     def get_full_logo_url(self):
         return "https://%s%s" % (settings.SITH_URL, self.logo.url)
@@ -228,28 +250,89 @@ class Club(models.Model):
             return False
         return sub.was_subscribed
 
-    _memberships = {}
-
-    def get_membership_for(self, user):
+    def get_membership_for(self, user: User) -> Optional["Membership"]:
         """
-        Returns the current membership the given user
+        Return the current membership the given user.
+        The result is cached.
         """
-        try:
-            return Club._memberships[self.id][user.id]
-        except:
-            m = self.members.filter(user=user.id).filter(end_date=None).first()
-            try:
-                Club._memberships[self.id][user.id] = m
-            except:
-                Club._memberships[self.id] = {}
-                Club._memberships[self.id][user.id] = m
-            return m
+        if user.is_anonymous:
+            return None
+        membership = cache.get(f"membership_{self.id}_{user.id}")
+        if membership == "not_member":
+            return None
+        if membership is None:
+            membership = self.members.filter(user=user, end_date=None).first()
+            if membership is None:
+                cache.set(f"membership_{self.id}_{user.id}", "not_member")
+            else:
+                cache.set(f"membership_{self.id}_{user.id}", membership)
+        return membership
 
     def has_rights_in_club(self, user):
         m = self.get_membership_for(user)
         return m is not None and m.role > settings.SITH_MAXIMUM_FREE_ROLE
 
 
+class MembershipQuerySet(models.QuerySet):
+    def ongoing(self) -> "MembershipQuerySet":
+        """
+        Filter all memberships which are not finished yet
+        """
+        # noinspection PyTypeChecker
+        return self.filter(Q(end_date=None) | Q(end_date__gte=timezone.now()))
+
+    def board(self) -> "MembershipQuerySet":
+        """
+        Filter all memberships where the user is/was in the board.
+
+        Be aware that users who were in the board in the past
+        are included, even if there are no more members.
+
+        If you want to get the users who are currently in the board,
+        mind combining this with the :meth:`ongoing` queryset method
+        """
+        # noinspection PyTypeChecker
+        return self.filter(role__gt=settings.SITH_MAXIMUM_FREE_ROLE)
+
+    def update(self, **kwargs):
+        """
+        Work just like the default Django's update() method,
+        but add a cache refresh for the elements of the queryset.
+
+        Be aware that this adds a db query to retrieve the updated objects
+        """
+        nb_rows = super().update(**kwargs)
+        if nb_rows > 0:
+            # if at least a row was affected, refresh the cache
+            for membership in self.all():
+                if membership.end_date is not None:
+                    cache.set(
+                        f"membership_{membership.club_id}_{membership.user_id}",
+                        "not_member",
+                    )
+                else:
+                    cache.set(
+                        f"membership_{membership.club_id}_{membership.user_id}",
+                        membership,
+                    )
+
+    def delete(self):
+        """
+        Work just like the default Django's delete() method,
+        but add a cache invalidation for the elements of the queryset
+        before the deletion.
+
+        Be aware that this adds a db query to retrieve the deleted element.
+        As this first query take place before the deletion operation,
+        it will be performed even if the deletion fails.
+        """
+        ids = list(self.values_list("club_id", "user_id"))
+        nb_rows, _ = super().delete()
+        if nb_rows > 0:
+            for club_id, user_id in ids:
+                cache.set(f"membership_{club_id}_{user_id}", "not_member")
+
+
 class Membership(models.Model):
     """
     The Membership class makes the connection between User and Clubs
@@ -289,6 +372,8 @@ class Membership(models.Model):
         _("description"), max_length=128, null=False, blank=True
     )
 
+    objects = MembershipQuerySet.as_manager()
+
     def __str__(self):
         return (
             self.club.name
@@ -303,24 +388,34 @@ class Membership(models.Model):
         """
         Method to see if that object can be super edited by the given user
         """
-        return user.is_in_group(settings.SITH_MAIN_BOARD_GROUP)
+        if user.is_anonymous:
+            return False
+        return user.is_board_member
 
-    def can_be_edited_by(self, user, membership=None):
+    def can_be_edited_by(self, user: User) -> bool:
         """
-        Method to see if that object can be edited by the given user
+        Check if that object can be edited by the given user
         """
-        if user.memberships:
-            if membership:  # This is for optimisation purpose
-                ms = membership
-            else:
-                ms = user.memberships.filter(club=self.club, end_date=None).first()
-            return (ms and ms.role >= self.role) or user.is_in_group(
-                settings.SITH_MAIN_BOARD_GROUP
-            )
-        return user.is_in_group(settings.SITH_MAIN_BOARD_GROUP)
+        if user.is_root or user.is_board_member:
+            return True
+        membership = self.club.get_membership_for(user)
+        if membership is not None and membership.role >= self.role:
+            return True
+        return False
 
     def get_absolute_url(self):
-        return reverse("club:club_members", kwargs={"club_id": self.club.id})
+        return reverse("club:club_members", kwargs={"club_id": self.club_id})
+
+    def save(self, *args, **kwargs):
+        super().save(*args, **kwargs)
+        if self.end_date is None:
+            cache.set(f"membership_{self.club_id}_{self.user_id}", self)
+        else:
+            cache.set(f"membership_{self.club_id}_{self.user_id}", "not_member")
+
+    def delete(self, *args, **kwargs):
+        super().delete(*args, **kwargs)
+        cache.delete(f"membership_{self.club_id}_{self.user_id}")
 
 
 class Mailing(models.Model):
@@ -373,14 +468,12 @@ class Mailing(models.Model):
         return self.email + "@" + settings.SITH_MAILING_DOMAIN
 
     def can_moderate(self, user):
-        return user.is_root or user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID)
+        return user.is_root or user.is_com_admin
 
     def is_owned_by(self, user):
-        return (
-            user.is_in_group(self)
-            or user.is_root
-            or user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID)
-        )
+        if user.is_anonymous:
+            return False
+        return user.is_root or user.is_com_admin
 
     def can_view(self, user):
         return self.club.has_rights_in_club(user)
@@ -388,9 +481,8 @@ class Mailing(models.Model):
     def can_be_edited_by(self, user):
         return self.club.has_rights_in_club(user)
 
-    def delete(self):
-        for sub in self.subscriptions.all():
-            sub.delete()
+    def delete(self, *args, **kwargs):
+        self.subscriptions.all().delete()
         super(Mailing, self).delete()
 
     def fetch_format(self):
@@ -463,10 +555,12 @@ class MailingSubscription(models.Model):
         super(MailingSubscription, self).clean()
 
     def is_owned_by(self, user):
+        if user.is_anonymous:
+            return False
         return (
             self.mailing.club.has_rights_in_club(user)
             or user.is_root
-            or self.user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID)
+            or self.user.is_com_admin
         )
 
     def can_be_edited_by(self, user):

club/templates/club/club_members.jinja

@@ -13,13 +13,15 @@
         {% endif %}
         <table>
             <thead>
-                <td>{% trans %}User{% endtrans %}</td>
-                <td>{% trans %}Role{% endtrans %}</td>
-                <td>{% trans %}Description{% endtrans %}</td>
-                <td>{% trans %}Since{% endtrans %}</td>
-                {% if users_old %}
-                    <td>{% trans %}Mark as old{% endtrans %}</td>
-                {% endif %}
+                <tr>
+                    <td>{% trans %}User{% endtrans %}</td>
+                    <td>{% trans %}Role{% endtrans %}</td>
+                    <td>{% trans %}Description{% endtrans %}</td>
+                    <td>{% trans %}Since{% endtrans %}</td>
+                    {% if users_old %}
+                        <td>{% trans %}Mark as old{% endtrans %}</td>
+                    {% endif %}
+                </tr>
             </thead>
             <tbody>
             {% for m in members %}

club/tests.py

@@ -13,378 +13,564 @@
 # OR WITHIN THE LOCAL FILE "LICENSE"
 #
 #
+from datetime import timedelta
 
 from django.conf import settings
+from django.core.cache import cache
 from django.test import TestCase
 from django.utils import timezone, html
+from django.utils.timezone import now, localtime
 from django.utils.translation import gettext as _
 from django.urls import reverse
 from django.core.management import call_command
-from django.core.exceptions import ValidationError, NON_FIELD_ERRORS
 
-from core.models import User
+from core.models import User, AnonymousUser
 from club.models import Club, Membership, Mailing
 from club.forms import MailingForm
-from sith.settings import SITH_BAR_MANAGER
-
-
-# Create your tests here.
+from sith.settings import SITH_BAR_MANAGER, SITH_MAIN_CLUB_ID
 
 
 class ClubTest(TestCase):
+    """
+    Set up data for test cases related to clubs and membership
+    The generated dataset is the one created by the populate command,
+    plus the following modifications :
+
+    - `self.club` is a dummy club recreated for each test
+    - `self.club` has two board members : skia (role 3) and comptable (role 10)
+    - `self.club` has one regular member : richard
+    - `self.club` has one former member : sli (who had role 2)
+    - None of the `self.club` members are in the AE club.
+    """
+
+    @classmethod
+    def setUpTestData(cls):
+        # subscribed users - initial members
+        cls.skia = User.objects.get(username="skia")
+        cls.richard = User.objects.get(username="rbatsbak")
+        cls.comptable = User.objects.get(username="comptable")
+        cls.sli = User.objects.get(username="sli")
+
+        # subscribed users - not initial members
+        cls.krophil = User.objects.get(username="krophil")
+        cls.subscriber = User.objects.get(username="subscriber")
+
+        # old subscriber
+        cls.old_subscriber = User.objects.get(username="old_subscriber")
+
+        # not subscribed
+        cls.public = User.objects.get(username="public")
+
+        cls.ae = Club.objects.filter(pk=SITH_MAIN_CLUB_ID)[0]
+
     def setUp(self):
-        call_command("populate")
-        self.skia = User.objects.filter(username="skia").first()
-        self.rbatsbak = User.objects.filter(username="rbatsbak").first()
-        self.guy = User.objects.filter(username="guy").first()
-        self.bdf = Club.objects.filter(unix_name=SITH_BAR_MANAGER["unix_name"]).first()
+        # by default, Skia is in the AE, which creates side effect
+        self.skia.memberships.all().delete()
 
-    def test_create_add_user_to_club_from_root_ok(self):
-        self.client.login(username="root", password="plop")
-        self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users": self.skia.id, "start_date": "12/06/2016", "role": 3},
+        # create a fake club
+        self.club = Club.objects.create(
+            name="Fake Club",
+            unix_name="fake-club",
+            address="5 rue de la République, 90000 Belfort",
         )
-        response = self.client.get(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id})
+        self.members_url = reverse(
+            "club:club_members", kwargs={"club_id": self.club.id}
         )
-        self.assertTrue(response.status_code == 200)
-        self.assertTrue(
-            "S&#39; Kia</a></td>\\n                    <td>Responsable info</td>"
-            in str(response.content)
+        a_month_ago = now() - timedelta(days=30)
+        yesterday = now() - timedelta(days=1)
+        Membership.objects.create(
+            club=self.club, user=self.skia, start_date=a_month_ago, role=3
+        )
+        Membership.objects.create(club=self.club, user=self.richard, role=1)
+        Membership.objects.create(
+            club=self.club, user=self.comptable, start_date=a_month_ago, role=10
         )
 
-    def test_create_add_multiple_user_to_club_from_root_ok(self):
+        # sli was a member but isn't anymore
+        Membership.objects.create(
+            club=self.club,
+            user=self.sli,
+            start_date=a_month_ago,
+            end_date=yesterday,
+            role=2,
+        )
+        cache.clear()
+
+
+class MembershipQuerySetTest(ClubTest):
+    def test_ongoing(self):
+        """
+        Test that the ongoing queryset method returns the memberships that
+        are not ended.
+        """
+        current_members = self.club.members.ongoing()
+        expected = [
+            self.skia.memberships.get(club=self.club),
+            self.comptable.memberships.get(club=self.club),
+            self.richard.memberships.get(club=self.club),
+        ]
+        self.assertEqual(len(current_members), len(expected))
+        for member in current_members:
+            self.assertIn(member, expected)
+
+    def test_board(self):
+        """
+        Test that the board queryset method returns the memberships
+        of user in the club board
+        """
+        board_members = list(self.club.members.board())
+        expected = [
+            self.skia.memberships.get(club=self.club),
+            self.comptable.memberships.get(club=self.club),
+            # sli is no more member, but he was in the board
+            self.sli.memberships.get(club=self.club),
+        ]
+        self.assertEqual(len(board_members), len(expected))
+        for member in board_members:
+            self.assertIn(member, expected)
+
+    def test_ongoing_board(self):
+        """
+        Test that combining ongoing and board returns users
+        who are currently board members of the club
+        """
+        members = list(self.club.members.ongoing().board())
+        expected = [
+            self.skia.memberships.get(club=self.club),
+            self.comptable.memberships.get(club=self.club),
+        ]
+        self.assertEqual(len(members), len(expected))
+        for member in members:
+            self.assertIn(member, expected)
+
+    def test_update_invalidate_cache(self):
+        """
+        Test that the `update` queryset method properly invalidate cache
+        """
+        mem_skia = self.skia.memberships.get(club=self.club)
+        cache.set(f"membership_{mem_skia.club_id}_{mem_skia.user_id}", mem_skia)
+        self.skia.memberships.update(end_date=localtime(now()).date())
+        self.assertEqual(
+            cache.get(f"membership_{mem_skia.club_id}_{mem_skia.user_id}"), "not_member"
+        )
+
+        mem_richard = self.richard.memberships.get(club=self.club)
+        cache.set(
+            f"membership_{mem_richard.club_id}_{mem_richard.user_id}", mem_richard
+        )
+        self.richard.memberships.update(role=5)
+        new_mem = self.richard.memberships.get(club=self.club)
+        self.assertNotEqual(new_mem, "not_member")
+        self.assertEqual(new_mem.role, 5)
+
+    def test_delete_invalidate_cache(self):
+        """
+        Test that the `delete` queryset properly invalidate cache
+        """
+
+        mem_skia = self.skia.memberships.get(club=self.club)
+        mem_comptable = self.comptable.memberships.get(club=self.club)
+        cache.set(f"membership_{mem_skia.club_id}_{mem_skia.user_id}", mem_skia)
+        cache.set(
+            f"membership_{mem_comptable.club_id}_{mem_comptable.user_id}", mem_comptable
+        )
+
+        # should delete the subscriptions of skia and comptable
+        self.club.members.ongoing().board().delete()
+
+        self.assertEqual(
+            cache.get(f"membership_{mem_skia.club_id}_{mem_skia.user_id}"), "not_member"
+        )
+        self.assertEqual(
+            cache.get(f"membership_{mem_comptable.club_id}_{mem_comptable.user_id}"),
+            "not_member",
+        )
+
+
+class ClubModelTest(ClubTest):
+    def assert_membership_just_started(self, user: User, role: int):
+        """
+        Assert that the given membership is active and started today
+        """
+        membership = user.memberships.ongoing().filter(club=self.club).first()
+        self.assertIsNotNone(membership)
+        self.assertEqual(localtime(now()).date(), membership.start_date)
+        self.assertIsNone(membership.end_date)
+        self.assertEqual(membership.role, role)
+        self.assertEqual(membership.club.get_membership_for(user), membership)
+        member_group = self.club.unix_name + settings.SITH_MEMBER_SUFFIX
+        board_group = self.club.unix_name + settings.SITH_BOARD_SUFFIX
+        self.assertTrue(user.is_in_group(name=member_group))
+        self.assertTrue(user.is_in_group(name=board_group))
+
+    def assert_membership_just_ended(self, user: User):
+        """
+        Assert that the given user have a membership which ended today
+        """
+        today = localtime(now()).date()
+        self.assertIsNotNone(
+            user.memberships.filter(club=self.club, end_date=today).first()
+        )
+        self.assertIsNone(self.club.get_membership_for(user))
+
+    def test_access_unauthorized(self):
+        """
+        Test that users who never subscribed and anonymous users
+        cannot see the page
+        """
+        response = self.client.post(self.members_url)
+        self.assertEqual(response.status_code, 403)
+
+        self.client.login(username="public", password="plop")
+        response = self.client.post(self.members_url)
+        self.assertEqual(response.status_code, 403)
+
+    def test_display(self):
+        """
+        Test that a GET request return a page where the requested
+        information are displayed.
+        """
+        self.client.login(username=self.skia.username, password="plop")
+        response = self.client.get(self.members_url)
+        self.assertEqual(response.status_code, 200)
+        expected_html = (
+            "<table><thead><tr>"
+            "<td>Utilisateur</td><td>Rôle</td><td>Description</td>"
+            "<td>Depuis</td><td>Marquer comme ancien</td>"
+            "</tr></thead><tbody>"
+        )
+        memberships = self.club.members.ongoing().order_by("-role")
+        input_id = 0
+        for membership in memberships.select_related("user"):
+            user = membership.user
+            expected_html += (
+                f"<tr><td><a href=\"{reverse('core:user_profile', args=[user.id])}\">"
+                f"{user.get_display_name()}</a></td>"
+                f"<td>{settings.SITH_CLUB_ROLES[membership.role]}</td>"
+                f"<td>{membership.description}</td>"
+                f"<td>{membership.start_date}</td><td>"
+            )
+            if membership.role <= 3:  # 3 is the role of skia
+                expected_html += (
+                    '<input type="checkbox" name="users_old" '
+                    f'value="{user.id}" '
+                    f'id="id_users_old_{input_id}">'
+                )
+                input_id += 1
+            expected_html += "</td></tr>"
+        expected_html += "</tbody></table>"
+        self.assertInHTML(expected_html, response.content.decode())
+
+    def test_root_add_one_club_member(self):
+        """
+        Test that root users can add members to clubs, one at a time
+        """
         self.client.login(username="root", password="plop")
-        self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
+        response = self.client.post(
+            self.members_url,
+            {"users": self.subscriber.id, "role": 3},
+        )
+        self.assertRedirects(response, self.members_url)
+        self.subscriber.refresh_from_db()
+        self.assert_membership_just_started(self.subscriber, role=3)
+
+    def test_root_add_multiple_club_member(self):
+        """
+        Test that root users can add multiple members at once to clubs
+        """
+        self.client.login(username="root", password="plop")
+        response = self.client.post(
+            self.members_url,
             {
-                "users": "|%d|%d|" % (self.skia.id, self.rbatsbak.id),
-                "start_date": "12/06/2016",
+                "users": f"|{self.subscriber.id}|{self.krophil.id}|",
                 "role": 3,
             },
         )
-        response = self.client.get(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id})
-        )
-        self.assertTrue(response.status_code == 200)
-        content = str(response.content)
-        self.assertTrue(
-            "S&#39; Kia</a></td>\\n                    <td>Responsable info</td>"
-            in content
-        )
-        self.assertTrue(
-            "Richard Batsbak</a></td>\\n                    <td>Responsable info</td>"
-            in content
-        )
+        self.assertRedirects(response, self.members_url)
+        self.subscriber.refresh_from_db()
+        self.assert_membership_just_started(self.subscriber, role=3)
+        self.assert_membership_just_started(self.krophil, role=3)
 
-    def test_create_add_user_to_club_from_root_fail_not_subscriber(self):
+    def test_add_unauthorized_members(self):
+        """
+        Test that users who are not currently subscribed
+        cannot be members of clubs.
+        """
         self.client.login(username="root", password="plop")
         response = self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users": self.guy.id, "start_date": "12/06/2016", "role": 3},
+            self.members_url,
+            {"users": self.public.id, "role": 1},
         )
-        self.assertTrue(response.status_code == 200)
+        self.assertIsNone(self.public.memberships.filter(club=self.club).first())
         self.assertTrue('<ul class="errorlist"><li>' in str(response.content))
-        response = self.client.get(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id})
-        )
-        self.assertFalse(
-            "Guy Carlier</a></td>\\n                    <td>Responsable info</td>"
-            in str(response.content)
-        )
 
-    def test_create_add_user_to_club_from_root_fail_already_in_club(self):
+        response = self.client.post(
+            self.members_url,
+            {"users": self.old_subscriber.id, "role": 1},
+        )
+        self.assertIsNone(self.public.memberships.filter(club=self.club).first())
+        self.assertIsNone(self.club.get_membership_for(self.public))
+        self.assertTrue('<ul class="errorlist"><li>' in str(response.content))
+
+    def test_add_members_already_members(self):
+        """
+        Test that users who are already members of a club
+        cannot be added again to this club
+        """
         self.client.login(username="root", password="plop")
+        current_membership = self.skia.memberships.ongoing().get(club=self.club)
+        nb_memberships = self.skia.memberships.count()
         self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users": self.skia.id, "start_date": "12/06/2016", "role": 3},
-        )
-        response = self.client.get(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id})
-        )
-        self.assertTrue(
-            "S&#39; Kia</a></td>\\n                    <td>Responsable info</td>"
-            in str(response.content)
-        )
-        response = self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users": self.skia.id, "start_date": "12/06/2016", "role": 4},
-        )
-        self.assertTrue(response.status_code == 200)
-        self.assertFalse(
-            "S&#39; Kia</a></td>\\n                <td>Secrétaire</td>"
-            in str(response.content)
+            self.members_url,
+            {"users": self.skia.id, "role": current_membership.role + 1},
         )
+        self.skia.refresh_from_db()
+        self.assertEqual(nb_memberships, self.skia.memberships.count())
+        new_membership = self.skia.memberships.ongoing().get(club=self.club)
+        self.assertEqual(current_membership, new_membership)
+        self.assertEqual(self.club.get_membership_for(self.skia), new_membership)
 
-    def test_create_add_user_non_existent_to_club_from_root_fail(self):
+    def test_add_not_existing_users(self):
+        """
+        Test that not existing users cannot be added in clubs.
+        If one user in the request is invalid, no membership creation at all
+        can take place.
+        """
         self.client.login(username="root", password="plop")
+        nb_memberships = self.club.members.count()
         response = self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users": [9999], "start_date": "12/06/2016", "role": 3},
+            self.members_url,
+            {"users": [9999], "role": 1},
         )
-        self.assertTrue(response.status_code == 200)
-        content = str(response.content)
-        self.assertTrue('<ul class="errorlist"><li>' in content)
-        self.assertFalse("<td>Responsable info</td>" in content)
-        self.client.login(username="root", password="plop")
+        self.assertContains(response, '<ul class="errorlist"><li>')
+        self.club.refresh_from_db()
+        self.assertEqual(self.club.members.count(), nb_memberships)
         response = self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
+            self.members_url,
             {
-                "users": "|%d|%d|" % (self.skia.id, 9999),
+                "users": f"|{self.subscriber.id}|{9999}|",
                 "start_date": "12/06/2016",
                 "role": 3,
             },
         )
-        self.assertTrue(response.status_code == 200)
-        content = str(response.content)
-        self.assertTrue('<ul class="errorlist"><li>' in content)
-        self.assertFalse("<td>Responsable info</td>" in content)
+        self.assertContains(response, '<ul class="errorlist"><li>')
+        self.club.refresh_from_db()
+        self.assertEqual(self.club.members.count(), nb_memberships)
 
-    def test_create_add_user_to_club_from_skia_ok(self):
-        self.client.login(username="root", password="plop")
-        self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users": self.skia.id, "start_date": "12/06/2016", "role": 10},
+    def test_president_add_members(self):
+        """
+        Test that the president of the club can add members
+        """
+        president = self.club.members.get(role=10).user
+        nb_club_membership = self.club.members.count()
+        nb_subscriber_memberships = self.subscriber.memberships.count()
+        self.client.login(username=president.username, password="plop")
+        response = self.client.post(
+            self.members_url,
+            {"users": self.subscriber.id, "role": 9},
         )
-        self.client.login(username="skia", password="plop")
-        self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users": self.rbatsbak.id, "start_date": "12/06/2016", "role": 9},
+        self.assertRedirects(response, self.members_url)
+        self.club.refresh_from_db()
+        self.subscriber.refresh_from_db()
+        self.assertEqual(self.club.members.count(), nb_club_membership + 1)
+        self.assertEqual(
+            self.subscriber.memberships.count(), nb_subscriber_memberships + 1
         )
-        response = self.client.get(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id})
+        self.assert_membership_just_started(self.subscriber, role=9)
+
+    def test_add_member_greater_role(self):
+        """
+        Test that a member of the club member cannot create
+        a membership with a greater role than its own.
+        """
+        self.client.login(username=self.skia.username, password="plop")
+        nb_memberships = self.club.members.count()
+        response = self.client.post(
+            self.members_url,
+            {"users": self.subscriber.id, "role": 10},
         )
         self.assertEqual(response.status_code, 200)
-        self.assertIn(
-            """Richard Batsbak</a></td>\n                    <td>Vice-Président⸱e</td>""",
+        self.assertInHTML(
+            "<li>Vous n'avez pas la permission de faire cela</li>",
             response.content.decode(),
         )
+        self.club.refresh_from_db()
+        self.assertEqual(nb_memberships, self.club.members.count())
+        self.assertIsNone(self.subscriber.memberships.filter(club=self.club).first())
 
-    def test_create_add_user_to_club_from_richard_fail(self):
-        self.client.login(username="root", password="plop")
-        self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users": self.rbatsbak.id, "start_date": "12/06/2016", "role": 3},
-        )
-        self.client.login(username="rbatsbak", password="plop")
-        response = self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users": self.skia.id, "start_date": "12/06/2016", "role": 10},
-        )
-        self.assertTrue(response.status_code == 200)
-        self.assertTrue(
-            "<li>Vous n&#x27;avez pas la permission de faire cela</li>"
-            in str(response.content)
-        )
-
-    def test_role_required_if_users_specified(self):
+    def test_add_member_without_role(self):
+        """
+        Test that trying to add members without specifying their role fails
+        """
         self.client.login(username="root", password="plop")
         response = self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users": self.rbatsbak.id, "start_date": "12/06/2016"},
+            self.members_url,
+            {"users": self.subscriber.id, "start_date": "12/06/2016"},
         )
         self.assertTrue(
             '<ul class="errorlist"><li>Vous devez choisir un r' in str(response.content)
         )
 
-    def test_mark_old_user_to_club_from_skia_ok(self):
-        self.client.login(username="root", password="plop")
-        self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {
-                "users": "|%d|%d|" % (self.skia.id, self.rbatsbak.id),
-                "start_date": "12/06/2016",
-                "role": 3,
-            },
-        )
+    def test_end_membership_self(self):
+        """
+        Test that a member can end its own membership
+        """
         self.client.login(username="skia", password="plop")
-        response = self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users_old": self.rbatsbak.id},
-        )
-        self.assertTrue(response.status_code == 302)
-
-        response = self.client.get(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id})
-        )
-        self.assertTrue(response.status_code == 200)
-        content = str(response.content)
-        self.assertFalse(
-            "Richard Batsbak</a></td>\\n                    <td>Responsable info</td>"
-            in content
-        )
-        self.assertTrue(
-            "S&#39; Kia</a></td>\\n                    <td>Responsable info</td>"
-            in content
-        )
-
-        # Skia is board member so he should be able to mark as old even without being in the club
-        response = self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
+        self.client.post(
+            self.members_url,
             {"users_old": self.skia.id},
         )
+        self.skia.refresh_from_db()
+        self.assert_membership_just_ended(self.skia)
+
+    def test_end_membership_lower_role(self):
+        """
+        Test that board members of the club can end memberships
+        of users with lower roles
+        """
+        # remainder : skia has role 3, comptable has role 10, richard has role 1
+        self.client.login(username=self.skia.username, password="plop")
+        response = self.client.post(
+            self.members_url,
+            {"users_old": self.richard.id},
+        )
+        self.assertRedirects(response, self.members_url)
+        self.club.refresh_from_db()
+        self.assert_membership_just_ended(self.richard)
+
+    def test_end_membership_higher_role(self):
+        """
+        Test that board members of the club cannot end memberships
+        of users with higher roles
+        """
+        membership = self.comptable.memberships.filter(club=self.club).first()
+        self.client.login(username=self.skia.username, password="plop")
+        self.client.post(
+            self.members_url,
+            {"users_old": self.comptable.id},
+        )
+        self.club.refresh_from_db()
+        new_membership = self.club.get_membership_for(self.comptable)
+        self.assertIsNotNone(new_membership)
+        self.assertEqual(new_membership, membership)
+
+        membership = self.comptable.memberships.filter(club=self.club).first()
+        self.assertIsNone(membership.end_date)
+
+    def test_end_membership_as_main_club_board(self):
+        """
+        Test that board members of the main club can end the membership
+        of anyone
+        """
+        # make subscriber a board member
+        self.subscriber.memberships.all().delete()
+        Membership.objects.create(club=self.ae, user=self.subscriber, role=3)
+
+        nb_memberships = self.club.members.count()
+        self.client.login(username=self.subscriber.username, password="plop")
+        response = self.client.post(
+            self.members_url,
+            {"users_old": self.comptable.id},
+        )
+        self.assertRedirects(response, self.members_url)
+        self.assert_membership_just_ended(self.comptable)
+        self.assertEqual(self.club.members.ongoing().count(), nb_memberships - 1)
+
+    def test_end_membership_as_root(self):
+        """
+        Test that root users can end the membership of anyone
+        """
+        nb_memberships = self.club.members.count()
         self.client.login(username="root", password="plop")
-        self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users": self.rbatsbak.id, "start_date": "12/06/2016", "role": 3},
-        )
-        self.client.login(username="skia", password="plop")
         response = self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users_old": self.rbatsbak.id},
-        )
-        self.assertFalse(
-            "Richard Batsbak</a></td>\\n                    <td>Responsable info</td>"
-            in str(response.content)
+            self.members_url,
+            {"users_old": [self.comptable.id]},
         )
+        self.assertRedirects(response, self.members_url)
+        self.assert_membership_just_ended(self.comptable)
+        self.assertEqual(self.club.members.ongoing().count(), nb_memberships - 1)
+        self.assertEqual(self.club.members.count(), nb_memberships)
 
-    def test_mark_old_multiple_users_from_skia_ok(self):
-        self.client.login(username="root", password="plop")
+    def test_end_membership_as_foreigner(self):
+        """
+        Test that users who are not in this club cannot end its memberships
+        """
+        nb_memberships = self.club.members.count()
+        membership = self.richard.memberships.filter(club=self.club).first()
+        self.client.login(username="subscriber", password="root")
         self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {
-                "users": "|%d|%d|" % (self.skia.id, self.rbatsbak.id),
-                "start_date": "12/06/2016",
-                "role": 3,
-            },
+            self.members_url,
+            {"users_old": [self.richard.id]},
         )
-        self.client.login(username="skia", password="plop")
-        response = self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users_old": [self.rbatsbak.id, self.skia.id]},
-        )
-        self.assertTrue(response.status_code == 302)
+        # nothing should have changed
+        new_mem = self.club.get_membership_for(self.richard)
+        self.assertIsNotNone(new_mem)
+        self.assertEqual(self.club.members.count(), nb_memberships)
+        self.assertEqual(membership, new_mem)
 
-        response = self.client.get(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id})
-        )
-        self.assertTrue(response.status_code == 200)
-        content = str(response.content)
-        self.assertFalse(
-            "Richard Batsbak</a></td>\\n                    <td>Responsable info</td>"
-            in content
-        )
-        self.assertFalse(
-            "S&#39; Kia</a></td>\\n                    <td>Responsable info</td>"
-            in content
-        )
+    def test_delete_remove_from_meta_group(self):
+        """
+        Test that when a club is deleted, all its members are removed from the
+        associated metagroup
+        """
+        memberships = self.club.members.select_related("user")
+        users = [membership.user for membership in memberships]
+        meta_group = self.club.unix_name + settings.SITH_MEMBER_SUFFIX
 
-    def test_mark_old_user_to_club_from_richard_ok(self):
-        self.client.login(username="root", password="plop")
-        self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {
-                "users": "|%d|%d|" % (self.skia.id, self.rbatsbak.id),
-                "start_date": "12/06/2016",
-                "role": 3,
-            },
-        )
+        self.club.delete()
+        for user in users:
+            self.assertFalse(user.is_in_group(name=meta_group))
 
-        # Test with equal rights
-        self.client.login(username="rbatsbak", password="plop")
-        response = self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users_old": self.skia.id},
-        )
-        self.assertTrue(response.status_code == 302)
+    def test_add_to_meta_group(self):
+        """
+        Test that when a membership begins, the user is added to the meta group
+        """
+        group_members = self.club.unix_name + settings.SITH_MEMBER_SUFFIX
+        board_members = self.club.unix_name + settings.SITH_BOARD_SUFFIX
+        self.assertFalse(self.subscriber.is_in_group(name=group_members))
+        self.assertFalse(self.subscriber.is_in_group(name=board_members))
+        Membership.objects.create(club=self.club, user=self.subscriber, role=3)
+        self.assertTrue(self.subscriber.is_in_group(name=group_members))
+        self.assertTrue(self.subscriber.is_in_group(name=board_members))
 
-        response = self.client.get(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id})
-        )
-        self.assertTrue(response.status_code == 200)
-        content = str(response.content)
-        self.assertTrue(
-            "Richard Batsbak</a></td>\\n                    <td>Responsable info</td>"
-            in content
-        )
-        self.assertFalse(
-            "S&#39; Kia</a></td>\\n                    <td>Responsable info</td>"
-            in content
-        )
+    def test_remove_from_meta_group(self):
+        """
+        Test that when a membership ends, the user is removed from meta group
+        """
+        group_members = self.club.unix_name + settings.SITH_MEMBER_SUFFIX
+        board_members = self.club.unix_name + settings.SITH_BOARD_SUFFIX
+        self.assertTrue(self.comptable.is_in_group(name=group_members))
+        self.assertTrue(self.comptable.is_in_group(name=board_members))
+        self.comptable.memberships.update(end_date=localtime(now()))
+        self.assertFalse(self.comptable.is_in_group(name=group_members))
+        self.assertFalse(self.comptable.is_in_group(name=board_members))
 
-        # Test with lower rights
-        self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users": self.skia.id, "start_date": "12/06/2016", "role": 0},
-        )
+    def test_club_owner(self):
+        """
+        Test that a club is owned only by board members of the main club
+        """
+        anonymous = AnonymousUser()
+        self.assertFalse(self.club.is_owned_by(anonymous))
+        self.assertFalse(self.club.is_owned_by(self.subscriber))
 
-        self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users_old": self.skia.id},
-        )
-        response = self.client.get(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id})
-        )
-        self.assertTrue(response.status_code == 200)
-        content = str(response.content)
-        self.assertTrue(
-            "Richard Batsbak</a></td>\\n                    <td>Responsable info</td>"
-            in content
-        )
-        self.assertFalse(
-            "S&#39; Kia</a></td>\\n                    <td>Curieux</td>" in content
-        )
-
-    def test_mark_old_user_to_club_from_richard_fail(self):
-        self.client.login(username="root", password="plop")
-        self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users": self.skia.id, "start_date": "12/06/2016", "role": 3},
-        )
-
-        # Test with richard outside of the club
-        self.client.login(username="rbatsbak", password="plop")
-        response = self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users_old": self.skia.id},
-        )
-        self.assertTrue(response.status_code == 200)
-
-        response = self.client.get(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id})
-        )
-        self.assertTrue(response.status_code == 200)
-        self.assertTrue(
-            "S&#39; Kia</a></td>\\n                    <td>Responsable info</td>"
-            in str(response.content)
-        )
-
-        # Test with lower rights
-        self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users": self.rbatsbak.id, "start_date": "12/06/2016", "role": 0},
-        )
-
-        self.client.post(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id}),
-            {"users_old": self.skia.id},
-        )
-        response = self.client.get(
-            reverse("club:club_members", kwargs={"club_id": self.bdf.id})
-        )
-        self.assertEqual(response.status_code, 200)
-        content = response.content.decode()
-        self.assertIn(
-            "Richard Batsbak</a></td>\n                    <td>Curieux⸱euse</td>",
-            content,
-        )
-        self.assertIn(
-            "S&#39; Kia</a></td>\n                    <td>Responsable info</td>",
-            content,
-        )
+        # make sli a board member
+        self.sli.memberships.all().delete()
+        Membership(club=self.ae, user=self.sli, role=3).save()
+        self.assertTrue(self.club.is_owned_by(self.sli))
 
 
 class MailingFormTest(TestCase):
     """Perform validation tests for MailingForm"""
 
+    @classmethod
+    def setUpTestData(cls):
+        cls.skia = User.objects.filter(username="skia").first()
+        cls.rbatsbak = User.objects.filter(username="rbatsbak").first()
+        cls.krophil = User.objects.filter(username="krophil").first()
+        cls.comunity = User.objects.filter(username="comunity").first()
+        cls.bdf = Club.objects.filter(unix_name=SITH_BAR_MANAGER["unix_name"]).first()
+
     def setUp(self):
-        call_command("populate")
-        self.skia = User.objects.filter(username="skia").first()
-        self.rbatsbak = User.objects.filter(username="rbatsbak").first()
-        self.krophil = User.objects.filter(username="krophil").first()
-        self.comunity = User.objects.filter(username="comunity").first()
-        self.bdf = Club.objects.filter(unix_name=SITH_BAR_MANAGER["unix_name"]).first()
         Membership(
             user=self.rbatsbak,
             club=self.bdf,
@@ -699,7 +885,6 @@ class ClubSellingViewTest(TestCase):
     """
 
     def setUp(self):
-        call_command("populate")
         self.ae = Club.objects.filter(unix_name="ae").first()
 
     def test_page_not_internal_error(self):

club/views.py

@@ -306,9 +306,7 @@ class ClubMembersView(ClubTabsMixin, CanViewMixin, DetailFormView):
         return resp
 
     def dispatch(self, request, *args, **kwargs):
-        self.members = (
-            self.get_object().members.filter(end_date=None).order_by("-role").all()
-        )
+        self.members = self.get_object().members.ongoing().order_by("-role")
         return super(ClubMembersView, self).dispatch(request, *args, **kwargs)
 
     def get_success_url(self, **kwargs):
@@ -443,7 +441,6 @@ class ClubSellingCSVView(ClubSellingView):
         return row
 
     def get(self, request, *args, **kwargs):
-
         self.object = self.get_object()
         kwargs = self.get_context_data(**kwargs)
 
@@ -706,7 +703,6 @@ class ClubMailingView(ClubTabsMixin, CanEditMixin, DetailFormView):
 
 
 class MailingDeleteView(CanEditMixin, DeleteView):
-
     model = Mailing
     template_name = "core/delete_confirm.jinja"
     pk_url_kwarg = "mailing_id"
@@ -724,7 +720,6 @@ class MailingDeleteView(CanEditMixin, DeleteView):
 
 
 class MailingSubscriptionDeleteView(CanEditMixin, DeleteView):
-
     model = MailingSubscription
     template_name = "core/delete_confirm.jinja"
     pk_url_kwarg = "mailing_subscription_id"

com/migrations/0001_initial.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = []
 
     operations = [

com/migrations/0002_news_newsdate.py

@@ -7,7 +7,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [
         ("club", "0005_auto_20161120_1149"),
         migrations.swappable_dependency(settings.AUTH_USER_MODEL),

com/migrations/0003_auto_20170115_2300.py

@@ -7,7 +7,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [
         ("club", "0006_auto_20161229_0040"),
         migrations.swappable_dependency(settings.AUTH_USER_MODEL),

com/migrations/0004_auto_20171221_1614.py

@@ -8,7 +8,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [
         ("club", "0010_auto_20170912_2028"),
         migrations.swappable_dependency(settings.AUTH_USER_MODEL),

com/migrations/0005_auto_20180318_2227.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("com", "0004_auto_20171221_1614")]
 
     operations = [

com/migrations/0006_remove_sith_index_page.py

@@ -6,7 +6,6 @@ from django.db import migrations
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("com", "0005_auto_20180318_2227")]
 
     operations = [migrations.RemoveField(model_name="sith", name="index_page")]

com/models.py

@@ -50,7 +50,9 @@ class Sith(models.Model):
     version = utils.get_git_revision_short_hash()
 
     def is_owned_by(self, user):
-        return user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID)
+        if user.is_anonymous:
+            return False
+        return user.is_com_admin
 
     def __str__(self):
         return "⛩ Sith ⛩"
@@ -92,13 +94,15 @@ class News(models.Model):
     )
 
     def is_owned_by(self, user):
-        return user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID) or user == self.author
+        if user.is_anonymous:
+            return False
+        return user.is_com_admin or user == self.author
 
     def can_be_edited_by(self, user):
-        return user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID)
+        return user.is_com_admin
 
     def can_be_viewed_by(self, user):
-        return self.is_moderated or user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID)
+        return self.is_moderated or user.is_com_admin
 
     def get_absolute_url(self):
         return reverse("com:news_detail", kwargs={"news_id": self.id})
@@ -243,7 +247,9 @@ class Weekmail(models.Model):
         return "Weekmail %s (sent: %s) - %s" % (self.id, self.sent, self.title)
 
     def is_owned_by(self, user):
-        return user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID)
+        if user.is_anonymous:
+            return False
+        return user.is_com_admin
 
 
 class WeekmailArticle(models.Model):
@@ -271,7 +277,9 @@ class WeekmailArticle(models.Model):
     rank = models.IntegerField(_("rank"), default=-1)
 
     def is_owned_by(self, user):
-        return user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID)
+        if user.is_anonymous:
+            return False
+        return user.is_com_admin
 
     def __str__(self):
         return "%s - %s (%s)" % (self.title, self.author, self.club)
@@ -287,7 +295,9 @@ class Screen(models.Model):
         )
 
     def is_owned_by(self, user):
-        return user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID)
+        if user.is_anonymous:
+            return False
+        return user.is_com_admin
 
     def __str__(self):
         return "%s" % (self.name)
@@ -340,12 +350,12 @@ class Poster(models.Model):
             raise ValidationError(_("Begin date should be before end date"))
 
     def is_owned_by(self, user):
-        return user.is_in_group(
-            settings.SITH_GROUP_COM_ADMIN_ID
-        ) or Club.objects.filter(id__in=user.clubs_with_rights)
+        if user.is_anonymous:
+            return False
+        return user.is_com_admin or len(user.clubs_with_rights) > 0
 
     def can_be_moderated_by(self, user):
-        return user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID)
+        return user.is_com_admin
 
     def get_display_name(self):
         return self.club.get_display_name()

com/templates/com/news_detail.jinja

@@ -35,7 +35,7 @@
         <p>{% trans %}Author: {% endtrans %}{{ user_profile_link(news.author) }}</p>
         {% if news.moderator %}
         <p>{% trans %}Moderator: {% endtrans %}{{ user_profile_link(news.moderator) }}</p>
-        {% elif user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID) %}
+        {% elif user.is_com_admin %}
         <p> <a href="{{ url('com:news_moderate', news_id=news.id) }}">{% trans %}Moderate{% endtrans %}</a></p>
         {% endif %}
         {% if user.can_edit(news) %}

com/templates/com/news_edit.jinja

@@ -49,7 +49,7 @@
     <p>{{ form.club.errors }}<label for="{{ form.club.name }}">{{ form.club.label }}</label> {{ form.club }}</p>
     <p>{{ form.summary.errors }}<label for="{{ form.summary.name }}">{{ form.summary.label }}</label> {{ form.summary }}</p>
     <p>{{ form.content.errors }}<label for="{{ form.content.name }}">{{ form.content.label }}</label> {{ form.content }}</p>
-    {% if user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID) %}
+    {% if user.is_com_admin %}
     <p>{{ form.automoderation.errors }}<label for="{{ form.automoderation.name }}">{{ form.automoderation.label }}</label>
         {{ form.automoderation }}</p>
     {% endif %}

com/templates/com/news_list.jinja

@@ -6,7 +6,7 @@
 {% endblock %}
 
 {% block content %}
-{% if user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID) %}
+{% if user.is_com_admin %}
 <div id="news_admin">
   <a class="button" href="{{ url('com:news_admin_list') }}">{% trans %}Administrate news{% endtrans %}</a>
 </div>

com/tests.py

@@ -13,22 +13,21 @@
 # OR WITHIN THE LOCAL FILE "LICENSE"
 #
 #
-
+from django.core.files.uploadedfile import SimpleUploadedFile
 from django.test import TestCase
 from django.conf import settings
 from django.urls import reverse
 from django.core.management import call_command
 from django.utils import html
+from django.utils.timezone import localtime, now
 from django.utils.translation import gettext as _
 
-
-from core.models import User, RealGroup
+from club.models import Club, Membership
+from com.models import Sith, News, Weekmail, WeekmailArticle, Poster
+from core.models import User, RealGroup, AnonymousUser
 
 
 class ComAlertTest(TestCase):
-    def setUp(self):
-        call_command("populate")
-
     def test_page_is_working(self):
         self.client.login(username="comunity", password="plop")
         response = self.client.get(reverse("com:alert_edit"))
@@ -37,9 +36,6 @@ class ComAlertTest(TestCase):
 
 
 class ComInfoTest(TestCase):
-    def setUp(self):
-        call_command("populate")
-
     def test_page_is_working(self):
         self.client.login(username="comunity", password="plop")
         response = self.client.get(reverse("com:info_edit"))
@@ -48,14 +44,16 @@ class ComInfoTest(TestCase):
 
 
 class ComTest(TestCase):
-    def setUp(self):
-        call_command("populate")
-        self.skia = User.objects.filter(username="skia").first()
-        self.com_group = RealGroup.objects.filter(
+    @classmethod
+    def setUpTestData(cls):
+        cls.skia = User.objects.filter(username="skia").first()
+        cls.com_group = RealGroup.objects.filter(
             id=settings.SITH_GROUP_COM_ADMIN_ID
         ).first()
-        self.skia.groups.set([self.com_group])
-        self.skia.save()
+        cls.skia.groups.set([cls.com_group])
+        cls.skia.save()
+
+    def setUp(self):
         self.client.login(username=self.skia.username, password="plop")
 
     def test_alert_msg(self):
@@ -114,3 +112,102 @@ class ComTest(TestCase):
                 _("You need an up to date subscription to access this content")
             ),
         )
+
+
+class SithTest(TestCase):
+    def test_sith_owner(self):
+        """
+        Test that the sith instance is owned by com admins
+        and nobody else
+        """
+        sith: Sith = Sith.objects.first()
+
+        com_admin = User.objects.get(username="comunity")
+        self.assertTrue(sith.is_owned_by(com_admin))
+
+        anonymous = AnonymousUser()
+        self.assertFalse(sith.is_owned_by(anonymous))
+
+        sli = User.objects.get(username="sli")
+        self.assertFalse(sith.is_owned_by(sli))
+
+
+class NewsTest(TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        cls.com_admin = User.objects.get(username="comunity")
+        new = News.objects.create(
+            title="dummy new",
+            summary="This is a dummy new",
+            content="Look at that beautiful dummy new",
+            author=User.objects.get(username="subscriber"),
+            club=Club.objects.first(),
+        )
+        cls.new = new
+        cls.author = new.author
+        cls.sli = User.objects.get(username="sli")
+        cls.anonymous = AnonymousUser()
+
+    def test_news_owner(self):
+        """
+        Test that news are owned by com admins
+        or by their author but nobody else
+        """
+
+        self.assertTrue(self.new.is_owned_by(self.com_admin))
+        self.assertTrue(self.new.is_owned_by(self.author))
+        self.assertFalse(self.new.is_owned_by(self.anonymous))
+        self.assertFalse(self.new.is_owned_by(self.sli))
+
+
+class WeekmailArticleTest(TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        cls.com_admin = User.objects.get(username="comunity")
+        author = User.objects.get(username="subscriber")
+        cls.article = WeekmailArticle.objects.create(
+            weekmail=Weekmail.objects.create(),
+            author=author,
+            title="title",
+            content="Some content",
+            club=Club.objects.first(),
+        )
+        cls.author = author
+        cls.sli = User.objects.get(username="sli")
+        cls.anonymous = AnonymousUser()
+
+    def test_weekmail_owner(self):
+        """
+        Test that weekmails are owned only by com admins
+        """
+        self.assertTrue(self.article.is_owned_by(self.com_admin))
+        self.assertFalse(self.article.is_owned_by(self.author))
+        self.assertFalse(self.article.is_owned_by(self.anonymous))
+        self.assertFalse(self.article.is_owned_by(self.sli))
+
+
+class PosterTest(TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        cls.com_admin = User.objects.get(username="comunity")
+        cls.poster = Poster.objects.create(
+            name="dummy",
+            file=SimpleUploadedFile("dummy.jpg", b"azertyuiop"),
+            club=Club.objects.first(),
+            date_begin=localtime(now()),
+        )
+        cls.sli = User.objects.get(username="sli")
+        cls.sli.memberships.all().delete()
+        Membership(user=cls.sli, club=Club.objects.first(), role=5).save()
+        cls.susbcriber = User.objects.get(username="subscriber")
+        cls.anonymous = AnonymousUser()
+
+    def test_poster_owner(self):
+        """
+        Test that poster are owned by com admins and board members in clubs
+        """
+        self.assertTrue(self.poster.is_owned_by(self.com_admin))
+        self.assertFalse(self.poster.is_owned_by(self.anonymous))
+
+        self.assertFalse(self.poster.is_owned_by(self.susbcriber))
+        self.assertTrue(self.poster.is_owned_by(self.sli))

com/views.py

@@ -146,7 +146,7 @@ class ComTabsMixin(TabedViewMixin):
 
 class IsComAdminMixin(View):
     def dispatch(self, request, *args, **kwargs):
-        if not (request.user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID)):
+        if not request.user.is_com_admin:
             raise PermissionDenied
         return super(IsComAdminMixin, self).dispatch(request, *args, **kwargs)
 
@@ -283,9 +283,7 @@ class NewsEditView(CanEditMixin, UpdateView):
 
     def form_valid(self, form):
         self.object = form.save()
-        if form.cleaned_data["automoderation"] and self.request.user.is_in_group(
-            settings.SITH_GROUP_COM_ADMIN_ID
-        ):
+        if form.cleaned_data["automoderation"] and self.request.user.is_com_admin:
             self.object.moderator = self.request.user
             self.object.is_moderated = True
             self.object.save()
@@ -333,9 +331,7 @@ class NewsCreateView(CanCreateMixin, CreateView):
 
     def form_valid(self, form):
         self.object = form.save()
-        if form.cleaned_data["automoderation"] and self.request.user.is_in_group(
-            settings.SITH_GROUP_COM_ADMIN_ID
-        ):
+        if form.cleaned_data["automoderation"] and self.request.user.is_com_admin:
             self.object.moderator = self.request.user
             self.object.is_moderated = True
             self.object.save()
@@ -617,10 +613,7 @@ class MailingListAdminView(ComTabsMixin, ListView):
     current_tab = "mailings"
 
     def dispatch(self, request, *args, **kwargs):
-        if not (
-            request.user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID)
-            or request.user.is_root
-        ):
+        if not (request.user.is_com_admin or request.user.is_root):
             raise PermissionDenied
         return super(MailingListAdminView, self).dispatch(request, *args, **kwargs)
 

core/apps.py

@@ -25,6 +25,7 @@
 import sys
 
 from django.apps import AppConfig
+from django.core.cache import cache
 from django.core.signals import request_started
 
 
@@ -33,26 +34,17 @@ class SithConfig(AppConfig):
     verbose_name = "Core app of the Sith"
 
     def ready(self):
-        from core.models import User
-        from club.models import Club
         from forum.models import Forum
+        import core.signals
 
-        def clear_cached_groups(**kwargs):
-            User._group_ids = {}
-            User._group_name = {}
+        cache.clear()
 
         def clear_cached_memberships(**kwargs):
-            User._club_memberships = {}
-            Club._memberships = {}
             Forum._club_memberships = {}
 
         print("Connecting signals!", file=sys.stderr)
-        request_started.connect(
-            clear_cached_groups, weak=False, dispatch_uid="clear_cached_groups"
-        )
         request_started.connect(
             clear_cached_memberships,
             weak=False,
             dispatch_uid="clear_cached_memberships",
         )
-        # TODO: there may be a need to add more cache clearing

core/management/commands/compilemessages.py

@@ -39,6 +39,5 @@ class Command(compilemessages.Command):
     """
 
     def handle(self, *args, **options):
-
         os.chdir("sith")
         super(Command, self).handle(*args, **options)

core/management/commands/compilestatic.py

@@ -60,7 +60,7 @@ class Command(BaseCommand):
 
     def compilescss(self, file):
         print("compiling %s" % file)
-        with (open(file.replace(".scss", ".css"), "w")) as newfile:
+        with open(file.replace(".scss", ".css"), "w") as newfile:
             newfile.write(self.compile(file))
 
     def removescss(self, file):
@@ -68,7 +68,6 @@ class Command(BaseCommand):
         os.remove(file)
 
     def handle(self, *args, **options):
-
         if os.path.isdir(settings.STATIC_ROOT):
             print("---- Compiling scss files ---")
             self.exec_on_folder(settings.STATIC_ROOT, self.compilescss)

core/management/commands/populate.py

@@ -155,12 +155,10 @@ class Command(BaseCommand):
         Counter(name="Eboutic", club=main_club, type="EBOUTIC").save()
         Counter(name="AE", club=main_club, type="OFFICE").save()
 
-        home_root.view_groups.set(
-            [Group.objects.filter(name=settings.SITH_MAIN_MEMBERS_GROUP).first()]
-        )
-        club_root.view_groups.set(
-            [Group.objects.filter(name=settings.SITH_MAIN_MEMBERS_GROUP).first()]
-        )
+        ae_members = Group.objects.get(name=settings.SITH_MAIN_MEMBERS_GROUP)
+
+        home_root.view_groups.set([ae_members])
+        club_root.view_groups.set([ae_members])
         home_root.save()
         club_root.save()
 
@@ -220,9 +218,7 @@ Welcome to the wiki page!
             )
             skia.set_password("plop")
             skia.save()
-            skia.view_groups = [
-                Group.objects.filter(name=settings.SITH_MAIN_MEMBERS_GROUP).first().id
-            ]
+            skia.view_groups = [ae_members.id]
             skia.save()
             skia_profile_path = (
                 root_path
@@ -261,9 +257,7 @@ Welcome to the wiki page!
             )
             public.set_password("plop")
             public.save()
-            public.view_groups = [
-                Group.objects.filter(name=settings.SITH_MAIN_MEMBERS_GROUP).first().id
-            ]
+            public.view_groups = [ae_members.id]
             public.save()
             # Adding user Subscriber
             subscriber = User(
@@ -277,9 +271,7 @@ Welcome to the wiki page!
             )
             subscriber.set_password("plop")
             subscriber.save()
-            subscriber.view_groups = [
-                Group.objects.filter(name=settings.SITH_MAIN_MEMBERS_GROUP).first().id
-            ]
+            subscriber.view_groups = [ae_members.id]
             subscriber.save()
             # Adding user old Subscriber
             old_subscriber = User(
@@ -293,9 +285,7 @@ Welcome to the wiki page!
             )
             old_subscriber.set_password("plop")
             old_subscriber.save()
-            old_subscriber.view_groups = [
-                Group.objects.filter(name=settings.SITH_MAIN_MEMBERS_GROUP).first().id
-            ]
+            old_subscriber.view_groups = [ae_members.id]
             old_subscriber.save()
             # Adding user Counter admin
             counter = User(
@@ -309,9 +299,7 @@ Welcome to the wiki page!
             )
             counter.set_password("plop")
             counter.save()
-            counter.view_groups = [
-                Group.objects.filter(name=settings.SITH_MAIN_MEMBERS_GROUP).first().id
-            ]
+            counter.view_groups = [ae_members.id]
             counter.groups.set(
                 [
                     Group.objects.filter(id=settings.SITH_GROUP_COUNTER_ADMIN_ID)
@@ -332,9 +320,7 @@ Welcome to the wiki page!
             )
             comptable.set_password("plop")
             comptable.save()
-            comptable.view_groups = [
-                Group.objects.filter(name=settings.SITH_MAIN_MEMBERS_GROUP).first().id
-            ]
+            comptable.view_groups = [ae_members.id]
             comptable.groups.set(
                 [
                     Group.objects.filter(id=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID)
@@ -355,9 +341,7 @@ Welcome to the wiki page!
             )
             u.set_password("plop")
             u.save()
-            u.view_groups = [
-                Group.objects.filter(name=settings.SITH_MAIN_MEMBERS_GROUP).first().id
-            ]
+            u.view_groups = [ae_members.id]
             u.save()
             # Adding user Richard Batsbak
             richard = User(
@@ -394,9 +378,7 @@ Welcome to the wiki page!
                 richard_profile.save()
                 richard.profile_pict = richard_profile
                 richard.save()
-            richard.view_groups = [
-                Group.objects.filter(name=settings.SITH_MAIN_MEMBERS_GROUP).first().id
-            ]
+            richard.view_groups = [ae_members.id]
             richard.save()
             # Adding syntax help page
             p = Page(name="Aide_sur_la_syntaxe")
@@ -428,7 +410,7 @@ Welcome to the wiki page!
             default_subscription = "un-semestre"
             # Root
             s = Subscription(
-                member=User.objects.filter(pk=root.pk).first(),
+                member=root,
                 subscription_type=default_subscription,
                 payment_method=settings.SITH_SUBSCRIPTION_PAYMENT_METHOD[0][0],
             )
@@ -528,7 +510,7 @@ Welcome to the wiki page!
             Club(
                 name="Woenzel'UT", unix_name="woenzel", address="Woenzel", parent=guyut
             ).save()
-            Membership(user=skia, club=main_club, role=3, description="").save()
+            Membership(user=skia, club=main_club, role=3).save()
             troll = Club(
                 name="Troll Penché",
                 unix_name="troll",
@@ -855,9 +837,7 @@ Welcome to the wiki page!
             )
             sli.set_password("plop")
             sli.save()
-            sli.view_groups = [
-                Group.objects.filter(name=settings.SITH_MAIN_MEMBERS_GROUP).first().id
-            ]
+            sli.view_groups = [ae_members.id]
             sli.save()
             sli_profile_path = (
                 root_path
@@ -934,7 +914,6 @@ Welcome to the wiki page!
             Membership(
                 user=comunity,
                 club=bar_club,
-                start_date=timezone.now(),
                 role=settings.SITH_CLUB_ROLES_ID["Board member"],
             ).save()
             # Adding user tutu

core/management/commands/setup.py

@@ -22,9 +22,6 @@ from django.core.management import call_command
 class Command(BaseCommand):
     help = "Set up a new instance of the Sith AE"
 
-    def add_arguments(self, parser):
-        parser.add_argument("--prod", action="store_true")
-
     def handle(self, *args, **options):
         root_path = os.path.dirname(
             os.path.dirname(os.path.dirname(os.path.dirname(__file__)))
@@ -40,7 +37,4 @@ class Command(BaseCommand):
         except Exception as e:
             repr(e)
         call_command("migrate")
-        if options["prod"]:
-            call_command("populate", "--prod")
-        else:
-            call_command("populate")
+        call_command("populate")

core/migrations/0001_initial.py

@@ -12,7 +12,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("auth", "0006_require_contenttypes_0002")]
 
     operations = [

core/migrations/0002_auto_20160831_0144.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0001_initial")]
 
     operations = [

core/migrations/0003_auto_20160902_1914.py

@@ -6,7 +6,6 @@ import django.core.validators
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0002_auto_20160831_0144")]
 
     operations = [

core/migrations/0004_user_godfathers.py

@@ -6,7 +6,6 @@ from django.conf import settings
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0003_auto_20160902_1914")]
 
     operations = [

core/migrations/0005_auto_20161105_1035.py

@@ -7,7 +7,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0004_user_godfathers")]
 
     operations = [

core/migrations/0006_auto_20161108_1703.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0005_auto_20161105_1035")]
 
     operations = [

core/migrations/0008_sithfile_asked_for_removal.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0006_auto_20161108_1703")]
 
     operations = [

core/migrations/0009_auto_20161120_1155.py

@@ -7,7 +7,6 @@ import core.models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0008_sithfile_asked_for_removal")]
 
     operations = [

core/migrations/0010_sithfile_is_in_sas.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0009_auto_20161120_1155")]
 
     operations = [

core/migrations/0011_auto_20161124_0848.py

@@ -7,7 +7,6 @@ import core.models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0010_sithfile_is_in_sas")]
 
     operations = [

core/migrations/0012_notification.py

@@ -8,7 +8,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0011_auto_20161124_0848")]
 
     operations = [

core/migrations/0013_auto_20161209_2338.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0012_notification")]
 
     operations = [

core/migrations/0014_auto_20161210_0009.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0013_auto_20161209_2338")]
 
     operations = [

core/migrations/0015_sithfile_moderator.py

@@ -7,7 +7,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0014_auto_20161210_0009")]
 
     operations = [

core/migrations/0016_auto_20161212_1922.py

@@ -7,7 +7,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0015_sithfile_moderator")]
 
     operations = [

core/migrations/0017_auto_20161220_1626.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0016_auto_20161212_1922")]
 
     operations = [

core/migrations/0018_auto_20161224_0211.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0017_auto_20161220_1626")]
 
     operations = [

core/migrations/0019_preferences_receive_weekmail.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0018_auto_20161224_0211")]
 
     operations = [

core/migrations/0020_auto_20170324_0917.py

@@ -6,7 +6,6 @@ import django.core.validators
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0019_preferences_receive_weekmail")]
 
     operations = [

core/migrations/0021_auto_20170822_1529.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0020_auto_20170324_0917")]
 
     operations = [

core/migrations/0022_auto_20170822_2232.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0021_auto_20170822_1529")]
 
     operations = [

core/migrations/0023_auto_20170902_1226.py

@@ -7,7 +7,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0022_auto_20170822_2232")]
 
     operations = [

core/migrations/0024_auto_20170906_1317.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0023_auto_20170902_1226")]
 
     operations = [

core/migrations/0025_auto_20170919_1521.py

@@ -6,7 +6,6 @@ import django.core.validators
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0024_auto_20170906_1317")]
 
     operations = [

core/migrations/0026_auto_20170926_1512.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0025_auto_20170919_1521")]
 
     operations = [

core/migrations/0027_gift.py

@@ -8,7 +8,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0026_auto_20170926_1512")]
 
     operations = [

core/migrations/0028_auto_20171216_2044.py

@@ -5,7 +5,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0027_gift")]
 
     operations = [

core/migrations/0029_auto_20180426_2013.py

@@ -7,7 +7,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0028_auto_20171216_2044")]
 
     operations = [

core/migrations/0030_auto_20190704_1500.py

@@ -6,7 +6,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0029_auto_20180426_2013")]
 
     operations = [

core/migrations/0031_auto_20190906_1615.py

@@ -6,7 +6,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0030_auto_20190704_1500")]
 
     operations = [

core/migrations/0032_auto_20190909_0043.py

@@ -6,7 +6,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0031_auto_20190906_1615")]
 
     operations = [

core/migrations/0033_auto_20191006_0049.py

@@ -5,7 +5,6 @@ from django.db import migrations
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0032_auto_20190909_0043")]
 
     operations = [

core/migrations/0034_operationlog.py

@@ -6,7 +6,6 @@ import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     dependencies = [
         ("core", "0033_auto_20191006_0049"),
     ]

core/migrations/0035_auto_20200216_1743.py

@@ -4,7 +4,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [
         ("core", "0034_operationlog"),
     ]

core/migrations/0036_auto_20211001_0248.py

@@ -4,7 +4,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0035_auto_20200216_1743")]
 
     operations = [

core/migrations/0037_auto_20211105_1708.py

@@ -4,7 +4,6 @@ from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
-
     dependencies = [("core", "0036_auto_20211001_0248")]
 
     operations = [

core/models.py

@@ -23,12 +23,12 @@
 #
 #
 import importlib
+from typing import Union, Optional, List
 
-from django.db import models
+from django.core.cache import cache
 from django.core.mail import send_mail
 from django.contrib.auth.models import (
     AbstractBaseUser,
-    PermissionsMixin,
     UserManager,
     Group as AuthGroup,
     GroupManager as AuthGroupManager,
@@ -40,7 +40,7 @@ from django.core import validators
 from django.core.exceptions import ValidationError, PermissionDenied
 from django.urls import reverse
 from django.conf import settings
-from django.db import transaction
+from django.db import models, transaction
 from django.contrib.staticfiles.storage import staticfiles_storage
 from django.utils.html import escape
 from django.utils.functional import cached_property
@@ -50,7 +50,7 @@ from core import utils
 
 from phonenumber_field.modelfields import PhoneNumberField
 
-from datetime import datetime, timedelta, date
+from datetime import timedelta, date
 
 import unicodedata
 
@@ -90,14 +90,24 @@ class Group(AuthGroup):
         """
         return reverse("core:group_list")
 
+    def save(self, *args, **kwargs):
+        super().save(*args, **kwargs)
+        cache.set(f"sith_group_{self.id}", self)
+        cache.set(f"sith_group_{self.name.replace(' ', '_')}", self)
+
+    def delete(self, *args, **kwargs):
+        super().delete(*args, **kwargs)
+        cache.delete(f"sith_group_{self.id}")
+        cache.delete(f"sith_group_{self.name.replace(' ', '_')}")
+
 
 class MetaGroup(Group):
     """
     MetaGroups are dynamically created groups.
-    Generaly used with clubs where creating a club creates two groups:
+    Generally used with clubs where creating a club creates two groups:
 
-        * club-SITH_BOARD_SUFFIX
-        * club-SITH_MEMBER_SUFFIX
+    * club-SITH_BOARD_SUFFIX
+    * club-SITH_MEMBER_SUFFIX
     """
 
     #: Assign a manager in a way that MetaGroup.objects only return groups with is_meta=False
@@ -110,6 +120,32 @@ class MetaGroup(Group):
         super(MetaGroup, self).__init__(*args, **kwargs)
         self.is_meta = True
 
+    @cached_property
+    def associated_club(self):
+        """
+        Return the group associated with this meta group
+
+        The result of this function is cached
+
+        :return: The associated club if it exists, else None
+        :rtype: club.models.Club | None
+        """
+        from club.models import Club
+
+        if self.name.endswith(settings.SITH_BOARD_SUFFIX):
+            # replace this with str.removesuffix as soon as Python
+            # is upgraded to 3.10
+            club_name = self.name[: -len(settings.SITH_BOARD_SUFFIX)]
+        elif self.name.endswith(settings.SITH_MEMBER_SUFFIX):
+            club_name = self.name[: -len(settings.SITH_MEMBER_SUFFIX)]
+        else:
+            return None
+        club = cache.get(f"sith_club_{club_name}")
+        if club is None:
+            club = Club.objects.filter(unix_name=club_name).first()
+            cache.set(f"sith_club_{club_name}", club)
+        return club
+
 
 class RealGroup(Group):
     """
@@ -134,6 +170,43 @@ def validate_promo(value):
         )
 
 
+def get_group(*, pk: int = None, name: str = None) -> Optional[Group]:
+    """
+    Search for a group by its primary key or its name.
+    Either one of the two must be set.
+
+    The result is cached for the default duration (should be 5 minutes).
+
+    :param pk: The primary key of the group
+    :param name: The name of the group
+    :return: The group if it exists, else None
+    :raises ValueError: If no group matches the criteria
+    """
+    if pk is None and name is None:
+        raise ValueError("Either pk or name must be set")
+    if name is not None:
+        name = name.replace(" ", "_")  # avoid errors with memcached backend
+    pk_or_name: Union[str, int] = pk if pk is not None else name
+    group = cache.get(f"sith_group_{pk_or_name}")
+    if group == "not_found":
+        # Using None as a cache value is a little bit tricky,
+        # so we use a special string to represent None
+        return None
+    elif group is not None:
+        return group
+    # if this point is reached, the group is not in cache
+    if pk is not None:
+        group = Group.objects.filter(pk=pk).first()
+    else:
+        group = Group.objects.filter(name=name).first()
+    if group is not None:
+        cache.set(f"sith_group_{group.id}", group)
+        cache.set(f"sith_group_{group.name.replace(' ', '_')}", group)
+    else:
+        cache.set(f"sith_group_{pk_or_name}", "not_found")
+    return group
+
+
 class User(AbstractBaseUser):
     """
     Defines the base user class, useable in every app
@@ -295,7 +368,6 @@ class User(AbstractBaseUser):
     objects = UserManager()
 
     USERNAME_FIELD = "username"
-    # REQUIRED_FIELDS = ['email']
 
     def promo_has_logo(self):
         return utils.file_exist("./core/static/core/img/promo_%02d.png" % self.promo)
@@ -336,94 +408,72 @@ class User(AbstractBaseUser):
         else:
             return 0
 
-    _club_memberships = {}
-    _group_names = {}
-    _group_ids = {}
+    def is_in_group(self, *, pk: int = None, name: str = None) -> bool:
+        """
+        Check if this user is in the given group.
+        Either a group id or a group name must be provided.
+        If both are passed, only the id will be considered.
 
-    def is_in_group(self, group_name):
-        """If the user is in the group passed in argument (as string or by id)"""
-        group_id = 0
-        g = None
-        if isinstance(group_name, int):  # Handle the case where group_name is an ID
-            if group_name in User._group_ids.keys():
-                g = User._group_ids[group_name]
-            else:
-                g = Group.objects.filter(id=group_name).first()
-                User._group_ids[group_name] = g
-        else:
-            if group_name in User._group_names.keys():
-                g = User._group_names[group_name]
-            else:
-                g = Group.objects.filter(name=group_name).first()
-                User._group_names[group_name] = g
-        if g:
-            group_name = g.name
-            group_id = g.id
+        The group will be fetched using the given parameter.
+        If no group is found, return False.
+        If a group is found, check if this user is in the latter.
+
+        :return: True if the user is the group, else False
+        """
+        if pk is not None:
+            group: Optional[Group] = get_group(pk=pk)
+        elif name is not None:
+            group: Optional[Group] = get_group(name=name)
         else:
+            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:
+        if group.id == settings.SITH_GROUP_PUBLIC_ID:
             return True
-        if group_id == settings.SITH_GROUP_SUBSCRIBERS_ID:
+        if group.id == settings.SITH_GROUP_SUBSCRIBERS_ID:
             return self.is_subscribed
-        if group_id == settings.SITH_GROUP_OLD_SUBSCRIBERS_ID:
+        if group.id == settings.SITH_GROUP_OLD_SUBSCRIBERS_ID:
             return self.was_subscribed
-        if (
-            group_name == settings.SITH_MAIN_MEMBERS_GROUP
-        ):  # We check the subscription if asked
-            return self.is_subscribed
-        if group_name[-len(settings.SITH_BOARD_SUFFIX) :] == settings.SITH_BOARD_SUFFIX:
-            name = group_name[: -len(settings.SITH_BOARD_SUFFIX)]
-            if name in User._club_memberships.keys():
-                mem = User._club_memberships[name]
-            else:
-                from club.models import Club
-
-                c = Club.objects.filter(unix_name=name).first()
-                mem = c.get_membership_for(self)
-                User._club_memberships[name] = mem
-            if mem:
-                return mem.role > settings.SITH_MAXIMUM_FREE_ROLE
-            return False
-        if (
-            group_name[-len(settings.SITH_MEMBER_SUFFIX) :]
-            == settings.SITH_MEMBER_SUFFIX
-        ):
-            name = group_name[: -len(settings.SITH_MEMBER_SUFFIX)]
-            if name in User._club_memberships.keys():
-                mem = User._club_memberships[name]
-            else:
-                from club.models import Club
-
-                c = Club.objects.filter(unix_name=name).first()
-                mem = c.get_membership_for(self)
-                User._club_memberships[name] = mem
-            if mem:
+        if group.id == settings.SITH_GROUP_ROOT_ID:
+            return self.is_root
+        if group.is_meta:
+            # check if this group is associated with a club
+            group.__class__ = MetaGroup
+            club = group.associated_club
+            if club is None:
+                return False
+            membership = club.get_membership_for(self)
+            if membership is None:
+                return False
+            if group.name.endswith(settings.SITH_MEMBER_SUFFIX):
                 return True
-            return False
-        if group_id == settings.SITH_GROUP_ROOT_ID and self.is_superuser:
+            return membership.role > settings.SITH_MAXIMUM_FREE_ROLE
+        return group in self.cached_groups
+
+    @property
+    def cached_groups(self) -> List[Group]:
+        """
+        Get the list of groups this user is in.
+        The result is cached for the default duration (should be 5 minutes)
+        :return: A list of all the groups this user is in
+        """
+        groups = cache.get(f"user_{self.id}_groups")
+        if groups is None:
+            groups = list(self.groups.all())
+            cache.set(f"user_{self.id}_groups", groups)
+        return groups
+
+    @cached_property
+    def is_root(self) -> bool:
+        if self.is_superuser:
             return True
-        return group_name in self.cached_groups_names
-
-    @cached_property
-    def cached_groups_names(self):
-        return [g.name for g in self.groups.all()]
-
-    @cached_property
-    def is_root(self):
-        return (
-            self.is_superuser
-            or self.groups.filter(id=settings.SITH_GROUP_ROOT_ID).exists()
-        )
+        root_id = settings.SITH_GROUP_ROOT_ID
+        return any(g.id == root_id for g in self.cached_groups)
 
     @cached_property
     def is_board_member(self):
-        from club.models import Club
-
-        return (
-            Club.objects.filter(unix_name=settings.SITH_MAIN_CLUB["unix_name"])
-            .first()
-            .has_rights_in_club(self)
-        )
+        main_club = settings.SITH_MAIN_CLUB["unix_name"]
+        return self.is_in_group(name=main_club + settings.SITH_BOARD_SUFFIX)
 
     @cached_property
     def can_read_subscription_history(self):
@@ -434,8 +484,8 @@ class User(AbstractBaseUser):
 
         for club in Club.objects.filter(
             id__in=settings.SITH_CAN_READ_SUBSCRIPTION_HISTORY
-        ).all():
-            if club.has_rights_in_club(self):
+        ):
+            if club in self.clubs_with_rights:
                 return True
         return False
 
@@ -443,10 +493,8 @@ class User(AbstractBaseUser):
     def can_create_subscription(self):
         from club.models import Club
 
-        for club in Club.objects.filter(
-            id__in=settings.SITH_CAN_CREATE_SUBSCRIPTIONS
-        ).all():
-            if club.has_rights_in_club(self):
+        for club in Club.objects.filter(id__in=settings.SITH_CAN_CREATE_SUBSCRIPTIONS):
+            if club in self.clubs_with_rights:
                 return True
         return False
 
@@ -464,11 +512,11 @@ class User(AbstractBaseUser):
 
     @cached_property
     def is_banned_alcohol(self):
-        return self.is_in_group(settings.SITH_GROUP_BANNED_ALCOHOL_ID)
+        return self.is_in_group(pk=settings.SITH_GROUP_BANNED_ALCOHOL_ID)
 
     @cached_property
     def is_banned_counter(self):
-        return self.is_in_group(settings.SITH_GROUP_BANNED_COUNTER_ID)
+        return self.is_in_group(pk=settings.SITH_GROUP_BANNED_COUNTER_ID)
 
     @cached_property
     def age(self) -> int:
@@ -598,9 +646,9 @@ class User(AbstractBaseUser):
         """
         if hasattr(obj, "is_owned_by") and obj.is_owned_by(self):
             return True
-        if hasattr(obj, "owner_group") and self.is_in_group(obj.owner_group.name):
+        if hasattr(obj, "owner_group") and self.is_in_group(pk=obj.owner_group.id):
             return True
-        if self.is_superuser or self.is_in_group(settings.SITH_GROUP_ROOT_ID):
+        if self.is_root:
             return True
         return False
 
@@ -611,8 +659,8 @@ class User(AbstractBaseUser):
         if hasattr(obj, "can_be_edited_by") and obj.can_be_edited_by(self):
             return True
         if hasattr(obj, "edit_groups"):
-            for g in obj.edit_groups.all():
-                if self.is_in_group(g.name):
+            for pk in obj.edit_groups.values_list("pk", flat=True):
+                if self.is_in_group(pk=pk):
                     return True
         if isinstance(obj, User) and obj == self:
             return True
@@ -627,15 +675,15 @@ class User(AbstractBaseUser):
         if hasattr(obj, "can_be_viewed_by") and obj.can_be_viewed_by(self):
             return True
         if hasattr(obj, "view_groups"):
-            for g in obj.view_groups.all():
-                if self.is_in_group(g.name):
+            for pk in obj.view_groups.values_list("pk", flat=True):
+                if self.is_in_group(pk=pk):
                     return True
         if self.can_edit(obj):
             return True
         return False
 
     def can_be_edited_by(self, user):
-        return user.is_in_group(settings.SITH_MAIN_BOARD_GROUP) or user.is_root
+        return user.is_root or user.is_board_member
 
     def can_be_viewed_by(self, user):
         return (user.was_subscribed and self.is_subscriber_viewable) or user.is_root
@@ -656,10 +704,6 @@ class User(AbstractBaseUser):
             escape(self.get_display_name()),
         )
 
-    @cached_property
-    def subscribed(self):
-        return self.is_in_group(settings.SITH_MAIN_MEMBERS_GROUP)
-
     @cached_property
     def preferences(self):
         try:
@@ -682,17 +726,16 @@ class User(AbstractBaseUser):
 
     @cached_property
     def clubs_with_rights(self):
-        return [
-            m.club.id
-            for m in self.memberships.filter(
-                models.Q(end_date__isnull=True) | models.Q(end_date__gte=timezone.now())
-            ).all()
-            if m.club.has_rights_in_club(self)
-        ]
+        """
+        :return: the list of clubs where the user has rights
+        :rtype: list[club.models.Club]
+        """
+        memberships = self.memberships.ongoing().board().select_related("club")
+        return [m.club for m in memberships]
 
     @cached_property
     def is_com_admin(self):
-        return self.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID)
+        return self.is_in_group(pk=settings.SITH_GROUP_COM_ADMIN_ID)
 
 
 class AnonymousUser(AuthAnonymousUser):
@@ -747,21 +790,18 @@ class AnonymousUser(AuthAnonymousUser):
     def favorite_topics(self):
         raise PermissionDenied
 
-    def is_in_group(self, group_name):
+    def is_in_group(self, *, pk: int = None, name: str = None) -> bool:
         """
-        The anonymous user is only the public group
+        The anonymous user is only in the public group
         """
-        group_id = 0
-        if isinstance(group_name, int):  # Handle the case where group_name is an ID
-            g = Group.objects.filter(id=group_name).first()
-            if g:
-                group_name = g.name
-                group_id = g.id
-            else:
-                return False
-        if group_id == settings.SITH_GROUP_PUBLIC_ID:
-            return True
-        return False
+        allowed_id = settings.SITH_GROUP_PUBLIC_ID
+        if pk is not None:
+            return pk == allowed_id
+        elif name is not None:
+            group = get_group(name=name)
+            return group is not None and group.id == allowed_id
+        else:
+            raise ValueError("You must either provide the id or the name of the group")
 
     def is_owner(self, obj):
         return False
@@ -879,14 +919,44 @@ class SithFile(models.Model):
     class Meta:
         verbose_name = _("file")
 
-    def is_owned_by(self, user):
-        if hasattr(self, "profile_of") and user.is_in_group(
-            settings.SITH_MAIN_BOARD_GROUP
+    def can_be_managed_by(self, user: User) -> bool:
+        """
+        Tell if the user can manage the file (edit, delete, etc.) or not.
+        Apply the following rules:
+            - If the file is not in the SAS nor in the profiles directory, it can be "managed" by anyone -> return True
+            - If the file is in the SAS, only the SAS admins (or roots) can manage it -> return True if the user is in the SAS admin group or is a root
+            - If the file is in the profiles directory, only the roots can manage it -> return True if the user is a root
+
+        :returns: True if the file is managed by the SAS or within the profiles directory, False otherwise
+        """
+
+        # If the file is not in the SAS nor in the profiles directory, it can be "managed" by anyone
+        profiles_dir = SithFile.objects.filter(name="profiles").first()
+        if not self.is_in_sas and not profiles_dir in self.get_parent_list():
+            return True
+
+        # If the file is in the SAS, only the SAS admins (or roots) can manage it
+        if self.is_in_sas and (
+            user.is_in_group(settings.SITH_GROUP_SAS_ADMIN_ID) or user.is_root
         ):
             return True
-        if user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID):
+
+        # If the file is in the profiles directory, only the roots can manage it
+        if profiles_dir in self.get_parent_list() and (
+            user.is_root or user.is_board_member
+        ):
             return True
-        if self.is_in_sas and user.is_in_group(settings.SITH_GROUP_SAS_ADMIN_ID):
+
+        return False
+
+    def is_owned_by(self, user):
+        if user.is_anonymous:
+            return False
+        if hasattr(self, "profile_of") and user.is_board_member:
+            return True
+        if user.is_com_admin:
+            return True
+        if self.is_in_sas and user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID):
             return True
         return user.id == self.owner.id
 
@@ -956,7 +1026,7 @@ class SithFile(models.Model):
 
     def save(self, *args, **kwargs):
         sas = SithFile.objects.filter(id=settings.SITH_SAS_ROOT_DIR_ID).first()
-        self.is_in_sas = sas in self.get_parent_list()
+        self.is_in_sas = sas in self.get_parent_list() or self == sas
         copy_rights = False
         if self.id is None:
             copy_rights = True
@@ -1090,12 +1160,6 @@ class SithFile(models.Model):
 
         return Album.objects.filter(id=self.id).first()
 
-    def __str__(self):
-        if self.is_folder:
-            return _("Folder: ") + self.name
-        else:
-            return _("File: ") + self.name
-
     def get_parent_list(self):
         l = []
         p = self.parent
@@ -1176,6 +1240,7 @@ class Page(models.Model):
     # Attention: this field may not be valid until you call save(). It's made for fast query, but don't rely on it when
     # playing with a Page object, use get_full_name() instead!
     _full_name = models.CharField(_("page name"), max_length=255, blank=True)
+
     # This function prevents generating migration upon settings change
     def get_default_owner_group():
         return settings.SITH_GROUP_ROOT_ID
@@ -1492,6 +1557,8 @@ class Gift(models.Model):
         return self.label
 
     def is_owned_by(self, user):
+        if user.is_anonymous:
+            return False
         return user.is_board_member or user.is_root
 
 

core/signals.py

@@ -0,0 +1,17 @@
+from django.core.cache import cache
+from django.db.models.signals import m2m_changed
+from django.dispatch import receiver
+
+from core.models import User
+
+
+@receiver(m2m_changed, sender=User.groups.through, dispatch_uid="user_groups_changed")
+def user_groups_changed(sender, instance: User, **kwargs):
+    """
+    Clear the cached clubs of the user
+    """
+    # As a m2m relationship doesn't live within the model
+    # but rather on an intermediary table, there is no
+    # model method to override, meaning we must use
+    # a signal to invalidate the cache when a user is removed from a club
+    cache.delete(f"user_{instance.id}_groups")

core/static/core/navbar.scss

@@ -43,7 +43,7 @@ nav.navbar {
       justify-content: center;
       display: flex !important;
     }
-
+    
     > .menu,
     > .link {
       box-sizing: border-box;
@@ -85,6 +85,22 @@ nav.navbar {
       background-color: rgba(0, 0, 0, .2);
     }
 
+    > .menu > .head,
+    > .link {
+      color: white;
+      padding: 10px 20px;
+      box-sizing: border-box;
+
+      @media (max-width: 500px) {
+        padding: 10px;
+      }
+    }
+
+    .link:hover,
+    .menu:hover {
+      background-color: rgba(0, 0, 0, .2);
+    }
+
     > .menu:hover > .content,
     > .menu > .head:hover + .content,
     > .menu > .content:hover {
@@ -130,5 +146,5 @@ nav.navbar {
         }
       }
     }
-  }
+  }  
 }

core/templates/core/file_detail.jinja

@@ -61,7 +61,7 @@
 {% if not file.home_of and not file.home_of_club and file.parent %}
 <p><a href="{{ url('core:file_delete', file_id=file.id, popup=popup) }}">{% trans %}Delete{% endtrans %}</a></p>
 {% endif %}
-{% if user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID) %}
+{% if user.is_com_admin %}
 <p><a href="{{ url('core:file_moderate', file_id=file.id) }}">{% trans %}Moderate{% endtrans %}</a></p>
 {% endif %}
 {% endblock %}

core/templates/core/user_clubs.jinja

@@ -67,7 +67,10 @@
     </tbody>
 </table>
 
-{% if profile.mailing_subscriptions.exists() and (profile.id == user.id or user.is_root or user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID)) %}
+{% if
+    profile.mailing_subscriptions.exists()
+    and (profile.id == user.id or user.is_root or user.is_com_admin)
+%}
     <h4>{% trans %}Subscribed mailing lists{% endtrans %}</h4>
     {% for sub in profile.mailing_subscriptions.all() %}
         <p>{{ sub.mailing.email }} <a href="{{ url('club:mailing_subscription_delete', mailing_subscription_id=sub.id) }}">{% trans %}Unsubscribe{% endtrans %}</a></p>

core/templates/core/user_detail.jinja

@@ -136,7 +136,12 @@
             </div>
         </div>
     </main>
-    {% if user.memberships.filter(end_date=None).exists() or user.is_in_group(settings.SITH_MAIN_BOARD_GROUP) or user == profile or user.is_in_group(settings.SITH_BAR_MANAGER_BOARD_GROUP) %}
+    {% if
+        user == profile
+        or user.memberships.ongoing().exists()
+        or user.is_board_member
+        or user.is_in_group(name=settings.SITH_BAR_MANAGER_BOARD_GROUP)
+    %}
         {# if the user is member of a club, he can view the subscription state #}
         <hr>
         {% if profile.is_subscribed %}

core/templates/core/user_edit.jinja

@@ -35,7 +35,7 @@
                 {%- else -%}
                     <em>{% trans %}To edit your profile picture, ask a member of the AE{% endtrans %}</em>
                 {%- endif -%}
-                {%- if user.is_in_group(settings.SITH_MAIN_BOARD_GROUP) and form.instance.profile_pict.id -%}
+                {%- if user.is_board_member and form.instance.profile_pict.id -%}
                     <a href="{{ url('core:file_delete', file_id=form.instance.profile_pict.id, popup='') }}">
                         {%- trans -%}Delete{%- endtrans -%}
                     </a>
@@ -55,7 +55,7 @@
             <div class="profile-picture-edit">
                 <p>{{ form["avatar_pict"].label }}</p>
                 {{ form["avatar_pict"] }}
-                {%- if user.is_in_group(settings.SITH_MAIN_BOARD_GROUP) and form.instance.avatar_pict.id -%}
+                {%- if user.is_board_member and form.instance.avatar_pict.id -%}
                     <a href="{{ url('core:file_delete', file_id=form.instance.avatar_pict.id, popup='') }}">
                         {%- trans -%}Delete{%- endtrans -%}
                     </a>
@@ -75,7 +75,7 @@
             <div class="profile-picture-edit">
                 <p>{{ form["scrub_pict"].label }}</p>
                 {{ form["scrub_pict"] }}
-                {%- if user.is_in_group(settings.SITH_MAIN_BOARD_GROUP) and form.instance.scrub_pict.id -%}
+                {%- if user.is_board_member and form.instance.scrub_pict.id -%}
                     <a href="{{ url('core:file_delete', file_id=form.instance.scrub_pict.id, popup='') }}">
                         {%- trans -%}Delete{%-endtrans -%}
                     </a>

core/templates/core/user_tools.jinja

@@ -35,18 +35,21 @@
         {% endif %}
 
         {% set is_admin_on_a_counter = false %}
-        {% for b in settings.SITH_COUNTER_BARS if user.is_in_group(b[1] + " admin") %}
+        {% for b in settings.SITH_COUNTER_BARS if user.is_in_group(name=b[1] + " admin") %}
             {% set is_admin_on_a_counter = true %}
         {% endfor %}
 
         {% if
-            user.is_in_group(settings.SITH_GROUP_COUNTER_ADMIN_ID) or user.is_root
-            or is_admin_on_a_counter
+            is_admin_on_a_counter
+            or user.is_root
+            or user.is_in_group(pk=settings.SITH_GROUP_COUNTER_ADMIN_ID)
         %}
             <div>
                 <h4>{% trans %}Counters{% endtrans %}</h4>
                 <ul>
-                    {% if user.is_in_group(settings.SITH_GROUP_COUNTER_ADMIN_ID) or user.is_root %}
+                    {% if user.is_root
+                       or user.is_in_group(pk=settings.SITH_GROUP_COUNTER_ADMIN_ID)
+                    %}
                         <li><a href="{{ url('counter:admin_list') }}">{% trans %}General counters management{% endtrans %}</a></li>
                         <li><a href="{{ url('counter:product_list') }}">{% trans %}Products management{% endtrans %}</a></li>
                         <li><a href="{{ url('counter:producttype_list') }}">{% trans %}Product types management{% endtrans %}</a></li>
@@ -57,7 +60,7 @@
                 </ul>
                 <ul>
                     {% for b in settings.SITH_COUNTER_BARS %}
-                        {% if user.is_in_group(b[1]+" admin") %}
+                        {% if user.is_in_group(name=b[1]+" admin") %}
                             {% set c = Counter.objects.filter(id=b[0]).first() %}
 
                             <li class="rows counter">
@@ -85,13 +88,16 @@
         {% endif %}
 
         {% if
-            user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) or user.is_root
-            or user.memberships.filter(end_date=None).filter(role__gte=7).all() | length > 10
+            user.is_root
+            or user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID)
+            or user.memberships.ongoing().filter(role__gte=7).count() > 10
         %}
             <div>
                 <h4>{% trans %}Accounting{% endtrans %}</h4>
                 <ul>
-                    {% if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) or user.is_root %}
+                    {% if user.is_root
+                       or user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID)
+                    %}
                         <li><a href="{{ url('accounting:refound_account') }}">{% trans %}Refound Account{% endtrans %}</a></li>
                         <li><a href="{{ url('accounting:bank_list') }}">{% trans %}General accounting{% endtrans %}</a></li>
                         <li><a href="{{ url('accounting:co_list') }}">{% trans %}Company list{% endtrans %}</a></li>
@@ -118,11 +124,15 @@
             </div>
         {% endif %}
 
-        {% if user.is_in_group(settings.SITH_GROUP_SAS_ADMIN_ID) or user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID) or user.is_root %}
+        {% if
+            user.is_root
+            or user.is_com_admin
+            or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID)
+        %}
             <div>
                 <h4>{% trans %}Communication{% endtrans %}</h4>
                 <ul>
-                    {% if user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID) or user.is_root %}
+                    {% if user.is_com_admin or user.is_root %}
                         <li><a href="{{ url('com:weekmail_article') }}">{% trans %}Create weekmail article{% endtrans %}</a></li>
                         <li><a href="{{ url('com:weekmail') }}">{% trans %}Weekmail{% endtrans %}</a></li>
                         <li><a href="{{ url('com:weekmail_destinations') }}">{% trans %}Weekmail destinations{% endtrans %}</a></li>
@@ -135,7 +145,7 @@
                         <li><a href="{{ url('com:poster_list') }}">{% trans %}Posters{% endtrans %}</a></li>
                         <li><a href="{{ url('com:screen_list') }}">{% trans %}Screens{% endtrans %}</a></li>
                     {% endif %}
-                    {% if user.is_in_group(settings.SITH_GROUP_SAS_ADMIN_ID) %}
+                    {% if user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID) %}
                         <li><a href="{{ url('sas:moderation') }}">{% trans %}Moderate pictures{% endtrans %}</a></li>
                     {% endif %}
                 </ul>
@@ -153,7 +163,10 @@
             </div>
         {% endif %}
 
-        {% if user.is_in_group(settings.SITH_GROUP_PEDAGOGY_ADMIN_ID) or user.is_root %}
+        {% if
+            user.is_root
+            or user.is_in_group(pk=settings.SITH_GROUP_PEDAGOGY_ADMIN_ID)
+        %}
             <div>
                 <h4>{% trans %}Pedagogy{% endtrans %}</h4>
                 <ul>

core/tests.py

@@ -15,13 +15,18 @@
 #
 
 import os
+from datetime import timedelta
 
+from django.core.cache import cache
 from django.test import Client, TestCase
 from django.urls import reverse
 from django.core.management import call_command
+from django.utils.timezone import now
 
-from core.models import User, Group, Page
+from club.models import Membership
+from core.models import User, Group, Page, AnonymousUser
 from core.markdown import markdown
+from sith import settings
 
 """
 to run these tests :
@@ -30,11 +35,9 @@ to run these tests :
 
 
 class UserRegistrationTest(TestCase):
-    def setUp(self):
-        try:
-            Group.objects.create(name="root")
-        except Exception as e:
-            print(e)
+    @classmethod
+    def setUpTestData(cls):
+        User.objects.all().delete()
 
     def test_register_user_form_ok(self):
         """
@@ -282,19 +285,8 @@ class MarkdownTest(TestCase):
 
 class PageHandlingTest(TestCase):
     def setUp(self):
-        self.root_group = Group.objects.create(name="root")
-        u = User(
-            username="root",
-            last_name="",
-            first_name="Bibou",
-            email="ae.info@utbm.fr",
-            date_of_birth="1942-06-12",
-            is_superuser=True,
-            is_staff=True,
-        )
-        u.set_password("plop")
-        u.save()
         self.client.login(username="root", password="plop")
+        self.root_group = Group.objects.get(name="Root")
 
     def test_create_page_ok(self):
         """
@@ -321,12 +313,20 @@ class PageHandlingTest(TestCase):
         """
         Should create a page correctly
         """
+        # remove all other pages to make sure there is no side effect
+        Page.objects.all().delete()
         self.client.post(
-            reverse("core:page_new"), {"parent": "", "name": "guy", "owner_group": "1"}
+            reverse("core:page_new"),
+            {"parent": "", "name": "guy", "owner_group": str(self.root_group.id)},
         )
+        page = Page.objects.first()
         response = self.client.post(
             reverse("core:page_new"),
-            {"parent": "1", "name": "bibou", "owner_group": "1"},
+            {
+                "parent": str(page.id),
+                "name": "bibou",
+                "owner_group": str(self.root_group.id),
+            },
         )
         response = self.client.get(
             reverse("core:page", kwargs={"page_name": "guy/bibou"})
@@ -392,9 +392,6 @@ http://git.an
 
 
 class UserToolsTest(TestCase):
-    def setUp(self):
-        call_command("populate")
-
     def test_anonymous_user_unauthorized(self):
         response = self.client.get(reverse("core:user_tools"))
         self.assertEqual(response.status_code, 403)
@@ -432,13 +429,12 @@ class UserToolsTest(TestCase):
 
 
 class FileHandlingTest(TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        cls.subscriber = User.objects.get(username="subscriber")
+
     def setUp(self):
-        try:
-            call_command("populate")
-            self.subscriber = User.objects.filter(username="subscriber").first()
-            self.client.login(username="subscriber", password="plop")
-        except Exception as e:
-            print(e)
+        self.client.login(username="subscriber", password="plop")
 
     def test_create_folder_home(self):
         response = self.client.post(
@@ -466,3 +462,150 @@ class FileHandlingTest(TestCase):
         )
         self.assertTrue(response.status_code == 200)
         self.assertTrue("ls</a>" in str(response.content))
+
+
+class UserIsInGroupTest(TestCase):
+    """
+    Test that the User.is_in_group() and AnonymousUser.is_in_group()
+    work as intended
+    """
+
+    @classmethod
+    def setUpTestData(cls):
+        from club.models import Club
+
+        cls.root_group = Group.objects.get(name="Root")
+        cls.public = Group.objects.get(name="Public")
+        cls.subscribers = Group.objects.get(name="Subscribers")
+        cls.old_subscribers = Group.objects.get(name="Old subscribers")
+        cls.accounting_admin = Group.objects.get(name="Accounting admin")
+        cls.com_admin = Group.objects.get(name="Communication admin")
+        cls.counter_admin = Group.objects.get(name="Counter admin")
+        cls.banned_alcohol = Group.objects.get(name="Banned from buying alcohol")
+        cls.banned_counters = Group.objects.get(name="Banned from counters")
+        cls.banned_subscription = Group.objects.get(name="Banned to subscribe")
+        cls.sas_admin = Group.objects.get(name="SAS admin")
+        cls.club = Club.objects.create(
+            name="Fake Club",
+            unix_name="fake-club",
+            address="Fake address",
+        )
+        cls.main_club = Club.objects.get(id=1)
+
+    def setUp(self) -> None:
+        self.toto = User.objects.create(
+            username="toto", first_name="a", last_name="b", email="a.b@toto.fr"
+        )
+        self.skia = User.objects.get(username="skia")
+
+    def assert_in_public_group(self, user):
+        self.assertTrue(user.is_in_group(pk=self.public.id))
+        self.assertTrue(user.is_in_group(name=self.public.name))
+
+    def assert_in_club_metagroups(self, user, club):
+        meta_groups_board = club.unix_name + settings.SITH_BOARD_SUFFIX
+        meta_groups_members = club.unix_name + settings.SITH_MEMBER_SUFFIX
+        self.assertFalse(user.is_in_group(name=meta_groups_board))
+        self.assertFalse(user.is_in_group(name=meta_groups_members))
+
+    def assert_only_in_public_group(self, user):
+        self.assert_in_public_group(user)
+        for group in (
+            self.root_group,
+            self.banned_counters,
+            self.accounting_admin,
+            self.sas_admin,
+            self.subscribers,
+            self.old_subscribers,
+        ):
+            self.assertFalse(user.is_in_group(pk=group.pk))
+            self.assertFalse(user.is_in_group(name=group.name))
+        meta_groups_board = self.club.unix_name + settings.SITH_BOARD_SUFFIX
+        meta_groups_members = self.club.unix_name + settings.SITH_MEMBER_SUFFIX
+        self.assertFalse(user.is_in_group(name=meta_groups_board))
+        self.assertFalse(user.is_in_group(name=meta_groups_members))
+
+    def test_anonymous_user(self):
+        """
+        Test that anonymous users are only in the public group
+        """
+        user = AnonymousUser()
+        self.assert_only_in_public_group(user)
+
+    def test_not_subscribed_user(self):
+        """
+        Test that users who never subscribed are only in the public group
+        """
+        self.assert_only_in_public_group(self.toto)
+
+    def test_wrong_parameter_fail(self):
+        """
+        Test that when neither the pk nor the name argument is given,
+        the function raises a ValueError
+        """
+        with self.assertRaises(ValueError):
+            self.toto.is_in_group()
+
+    def test_number_queries(self):
+        """
+        Test that the number of db queries is stable
+        and that less queries are made when making a new call
+        """
+        # make sure Skia is in at least one group
+        self.skia.groups.add(Group.objects.first().pk)
+        skia_groups = self.skia.groups.all()
+
+        group_in = skia_groups.first()
+        cache.clear()
+        # Test when the user is in the group
+        with self.assertNumQueries(2):
+            self.skia.is_in_group(pk=group_in.id)
+        with self.assertNumQueries(0):
+            self.skia.is_in_group(pk=group_in.id)
+
+        ids = skia_groups.values_list("pk", flat=True)
+        group_not_in = Group.objects.exclude(pk__in=ids).first()
+        cache.clear()
+        # Test when the user is not in the group
+        with self.assertNumQueries(2):
+            self.skia.is_in_group(pk=group_not_in.id)
+        with self.assertNumQueries(0):
+            self.skia.is_in_group(pk=group_not_in.id)
+
+    def test_cache_properly_cleared_membership(self):
+        """
+        Test that when the membership of a user end,
+        the cache is properly invalidated
+        """
+        membership = Membership.objects.create(
+            club=self.club, user=self.toto, end_date=None
+        )
+        meta_groups_members = self.club.unix_name + settings.SITH_MEMBER_SUFFIX
+        cache.clear()
+        self.assertTrue(self.toto.is_in_group(name=meta_groups_members))
+        self.assertEqual(
+            membership, cache.get(f"membership_{self.club.id}_{self.toto.id}")
+        )
+        membership.end_date = now() - timedelta(minutes=5)
+        membership.save()
+        cached_membership = cache.get(f"membership_{self.club.id}_{self.toto.id}")
+        self.assertEqual(cached_membership, "not_member")
+        self.assertFalse(self.toto.is_in_group(name=meta_groups_members))
+
+    def test_cache_properly_cleared_group(self):
+        """
+        Test that when a user is removed from a group,
+        the is_in_group_method return False when calling it again
+        """
+        self.toto.groups.add(self.com_admin.pk)
+        self.assertTrue(self.toto.is_in_group(pk=self.com_admin.pk))
+
+        self.toto.groups.remove(self.com_admin.pk)
+        self.assertFalse(self.toto.is_in_group(pk=self.com_admin.pk))
+
+    def test_not_existing_group(self):
+        """
+        Test that searching for a not existing group
+        returns False
+        """
+        self.assertFalse(self.skia.is_in_group(name="This doesn't exist"))