Merge pull request #821 from ae-utbm/taiste

Python upgrade and bugfixes

.github/actions/setup_project/action.yml

@@ -16,16 +16,16 @@ runs:
       shell: bash
 
     - name: Set up python
-      uses: actions/setup-python@v4
+      uses: actions/setup-python@v5
       with:
-        python-version: "3.10"
+        python-version: "3.12"
 
     - name: Load cached Poetry installation
       id: cached-poetry
       uses: actions/cache@v3
       with:
         path: ~/.local
-        key: poetry-0  # increment to reset cache
+        key: poetry-1  # increment to reset cache
 
     - name: Install Poetry
       if: steps.cached-poetry.outputs.cache-hit != 'true'

.github/workflows/deploy.yml

@@ -36,7 +36,8 @@ jobs:
           export PATH="/home/sith/.local/bin:$PATH"
           pushd ${{secrets.SITH_PATH}}
 
-          git pull
+          git fetch
+          git reset --hard origin/master
           poetry install --with prod --without docs,tests
           poetry run ./manage.py install_xapian
           poetry run ./manage.py migrate

.github/workflows/deploy_docs.yml

@@ -9,7 +9,7 @@ jobs:
   deploy:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - uses: ./.github/actions/setup_project
       - run: echo "cache_id=$(date --utc '+%V')" >> $GITHUB_ENV
       - uses: actions/cache@v3

.github/workflows/taiste.yml

@@ -35,7 +35,8 @@ jobs:
           export PATH="$HOME/.poetry/bin:$PATH"
           pushd ${{secrets.SITH_PATH}}
 
-          git pull
+          git fetch
+          git reset --hard origin/taiste
           poetry install --with prod --without docs,tests
           poetry run ./manage.py install_xapian
           poetry run ./manage.py migrate

.pre-commit-config.yaml

@@ -1,7 +1,7 @@
 repos:
   - repo: https://github.com/astral-sh/ruff-pre-commit
     # Ruff version.
-    rev: v0.5.5
+    rev: v0.5.7
     hooks:
       - id: ruff  # just check the code, and print the errors
       - id: ruff  # actually fix the fixable errors, but print nothing

club/models.py

@@ -23,6 +23,8 @@
 #
 from __future__ import annotations
 
+from typing import Self
+
 from django.conf import settings
 from django.core import validators
 from django.core.cache import cache
@@ -265,12 +267,11 @@ def has_rights_in_club(self, user):
 
 
 class MembershipQuerySet(models.QuerySet):
-    def ongoing(self) -> "MembershipQuerySet":
+    def ongoing(self) -> Self:
         """Filter all memberships which are not finished yet."""
-        # noinspection PyTypeChecker
-        return self.filter(Q(end_date=None) | Q(end_date__gte=timezone.now()))
+        return self.filter(Q(end_date=None) | Q(end_date__gt=timezone.now().date()))
 
-    def board(self) -> "MembershipQuerySet":
+    def board(self) -> Self:
         """Filter all memberships where the user is/was in the board.
 
         Be aware that users who were in the board in the past
@@ -279,7 +280,6 @@ def board(self) -> "MembershipQuerySet":
         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):

club/tests.py

@@ -24,6 +24,7 @@
 
 from club.forms import MailingForm
 from club.models import Club, Mailing, Membership
+from core.baker_recipes import subscriber_user
 from core.models import AnonymousUser, User
 from sith.settings import SITH_BAR_MANAGER, SITH_MAIN_CLUB_ID
 
@@ -106,6 +107,18 @@ def test_ongoing(self):
         expected.sort(key=lambda i: i.id)
         assert current_members == expected
 
+    def test_ongoing_with_membership_ending_today(self):
+        """Test that a membership ending the present day is considered as ended."""
+        today = timezone.now().date()
+        self.richard.memberships.filter(club=self.club).update(end_date=today)
+        current_members = list(self.club.members.ongoing().order_by("id"))
+        expected = [
+            self.skia.memberships.get(club=self.club),
+            self.comptable.memberships.get(club=self.club),
+        ]
+        expected.sort(key=lambda i: i.id)
+        assert current_members == expected
+
     def test_board(self):
         """Test that the board queryset method returns the memberships
         of user in the club board.
@@ -422,11 +435,11 @@ def test_end_membership_as_main_club_board(self):
         of anyone.
         """
         # make subscriber a board member
-        self.subscriber.memberships.all().delete()
-        Membership.objects.create(club=self.ae, user=self.subscriber, role=3)
+        subscriber = subscriber_user.make()
+        Membership.objects.create(club=self.ae, user=subscriber, role=3)
 
-        nb_memberships = self.club.members.count()
-        self.client.force_login(self.subscriber)
+        nb_memberships = self.club.members.ongoing().count()
+        self.client.force_login(subscriber)
         response = self.client.post(
             self.members_url,
             {"users_old": self.comptable.id},
@@ -437,7 +450,7 @@ def test_end_membership_as_main_club_board(self):
 
     def test_end_membership_as_root(self):
         """Test that root users can end the membership of anyone."""
-        nb_memberships = self.club.members.count()
+        nb_memberships = self.club.members.ongoing().count()
         self.client.force_login(self.root)
         response = self.client.post(
             self.members_url,
@@ -446,7 +459,6 @@ def test_end_membership_as_root(self):
         self.assertRedirects(response, self.members_url)
         self.assert_membership_ended_today(self.comptable)
         assert self.club.members.ongoing().count() == nb_memberships - 1
-        assert self.club.members.count() == nb_memberships
 
     def test_end_membership_as_foreigner(self):
         """Test that users who are not in this club cannot end its memberships."""

core/management/commands/install_xapian.sh

@@ -32,13 +32,13 @@ tar xf "${BINDINGS}.tar.xz"
 # install
 echo "Installing Xapian-core..."
 cd "$VIRTUAL_ENV/packages/${CORE}" || exit 1
-./configure --prefix="$VIRTUAL_ENV" && make && make install
+./configure --prefix="$VIRTUAL_ENV" && make -j"$(nproc)" && make install
 
 PYTHON_FLAG=--with-python3
 
 echo "Installing Xapian-bindings..."
 cd "$VIRTUAL_ENV/packages/${BINDINGS}" || exit 1
-./configure --prefix="$VIRTUAL_ENV" $PYTHON_FLAG XAPIAN_CONFIG="$VIRTUAL_ENV/bin/xapian-config" && make && make install
+./configure --prefix="$VIRTUAL_ENV" $PYTHON_FLAG XAPIAN_CONFIG="$VIRTUAL_ENV/bin/xapian-config" && make -j"$(nproc)" && make install
 
 # clean
 rm -rf "$VIRTUAL_ENV/packages"

core/models.py

@@ -944,40 +944,15 @@ def save(self, *args, **kwargs):
                     param="1",
                 ).save()
 
-    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 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
-
-        return False
-
     def is_owned_by(self, user):
         if user.is_anonymous:
             return False
-        if hasattr(self, "profile_of") and user.is_board_member:
+        if user.is_root:
             return True
+        if hasattr(self, "profile_of"):
+            # if the `profile_of` attribute is set, this file is a profile picture
+            # and profile pictures may only be edited by board members
+            return user.is_board_member
         if user.is_com_admin:
             return True
         if self.is_in_sas and user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID):
@@ -993,7 +968,7 @@ def can_be_viewed_by(self, user):
             return user.can_view(self.scrub_of)
         return False
 
-    def delete(self):
+    def delete(self, *args, **kwargs):
         for c in self.children.all():
             c.delete()
         self.file.delete()

core/views/files.py

@@ -190,13 +190,6 @@ class FileEditView(CanEditMixin, UpdateView):
     template_name = "core/file_edit.jinja"
     context_object_name = "file"
 
-    def get(self, request, *args, **kwargs):
-        self.object = self.get_object()
-        if not self.object.can_be_managed_by(request.user):
-            raise PermissionDenied
-
-        return super().get(request, *args, **kwargs)
-
     def get_form_class(self):
         fields = ["name", "is_moderated"]
         if self.object.is_file:
@@ -242,13 +235,6 @@ class FileEditPropView(CanEditPropMixin, UpdateView):
     context_object_name = "file"
     form_class = FileEditPropForm
 
-    def get(self, request, *args, **kwargs):
-        self.object = self.get_object()
-        if not self.object.can_be_managed_by(request.user):
-            raise PermissionDenied
-
-        return super().get(request, *args, **kwargs)
-
     def get_form(self, form_class=None):
         form = super().get_form(form_class)
         form.fields["parent"].queryset = SithFile.objects.filter(is_folder=True)
@@ -322,9 +308,6 @@ def handle_clipboard(request, obj):
 
     def get(self, request, *args, **kwargs):
         self.form = self.get_form()
-        if not self.object.can_be_managed_by(request.user):
-            raise PermissionDenied
-
         if "clipboard" not in request.session.keys():
             request.session["clipboard"] = []
         return super().get(request, *args, **kwargs)
@@ -372,13 +355,6 @@ class FileDeleteView(CanEditPropMixin, DeleteView):
     template_name = "core/file_delete_confirm.jinja"
     context_object_name = "file"
 
-    def get(self, request, *args, **kwargs):
-        self.object = self.get_object()
-        if not self.object.can_be_managed_by(request.user):
-            raise PermissionDenied
-
-        return super().get(request, *args, **kwargs)
-
     def get_success_url(self):
         self.object.file.delete()  # Doing it here or overloading delete() is the same, so let's do it here
         if "next" in self.request.GET.keys():
@@ -416,6 +392,7 @@ class FileModerateView(CanEditPropMixin, SingleObjectMixin):
     model = SithFile
     pk_url_kwarg = "file_id"
 
+    # FIXME : wrong http method. This should be a POST or a DELETE request
     def get(self, request, *args, **kwargs):
         self.object = self.get_object()
         self.object.is_moderated = True

core/views/user.py

@@ -561,6 +561,8 @@ class UserListView(ListView, CanEditPropMixin):
     template_name = "core/user_list.jinja"
 
 
+# FIXME: the edit_once fields aren't displayed to the user (as expected).
+#  However, if the user re-add them manually in the form, they are saved.
 class UserUpdateProfileView(UserTabsMixin, CanEditMixin, UpdateView):
     """Edit a user's profile."""
 

counter/models.py

@@ -362,7 +362,6 @@ def annotate_has_barman(self, user: User) -> CounterQuerySet:
             ```
         """
         subquery = user.counters.filter(pk=OuterRef("pk"))
-        # noinspection PyTypeChecker
         return self.annotate(has_annotated_barman=Exists(subquery))