{% trans %}Manage simplified types{% endtrans %}
{% trans %}Manage accounting types{% endtrans %}
{% trans %}New bank account{% endtrans %}
diff --git a/accounting/templates/accounting/club_account_details.jinja b/accounting/templates/accounting/club_account_details.jinja index 13626950..f9ceec23 100644 --- a/accounting/templates/accounting/club_account_details.jinja +++ b/accounting/templates/accounting/club_account_details.jinja @@ -16,7 +16,7 @@ {% if user.is_root and not object.journals.exists() %} {% trans %}Delete{% endtrans %} {% endif %} - {% if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %} + {% if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %}{% trans %}New label{% endtrans %}
{% endif %}{% trans %}Label list{% endtrans %}
@@ -56,7 +56,7 @@ {% endif %}{% trans %}Create new company{% endtrans %}
{% endif %} - - +| - | {% endif %}- {% 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 %} - {% trans %}Edit{% endtrans %} - {% 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 %} + {% trans %}Edit{% endtrans %} + {% endif %} {% endif %} | {% trans %}Generate{% endtrans %} | diff --git a/accounting/templates/accounting/label_list.jinja b/accounting/templates/accounting/label_list.jinja index f790c608..c15a9373 100644 --- a/accounting/templates/accounting/label_list.jinja +++ b/accounting/templates/accounting/label_list.jinja @@ -13,7 +13,7 @@
| {% trans %}User{% endtrans %} | -{% trans %}Role{% endtrans %} | -{% trans %}Description{% endtrans %} | -{% trans %}Since{% endtrans %} | - {% if users_old %} -{% trans %}Mark as old{% endtrans %} | - {% endif %} +
| {% trans %}User{% endtrans %} | +{% trans %}Role{% endtrans %} | +{% trans %}Description{% endtrans %} | +{% trans %}Since{% endtrans %} | + {% if users_old %} +{% trans %}Mark as old{% endtrans %} | + {% endif %} +Responsable info | " - 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 = ( + "
| Utilisateur | Rôle | Description | " + "Depuis | Marquer comme ancien | " + "
| " + f"{user.get_display_name()} | " + f"{settings.SITH_CLUB_ROLES[membership.role]} | " + f"{membership.description} | " + f"{membership.start_date} | " + ) + if membership.role <= 3: # 3 is the role of skia + expected_html += ( + '' + ) + input_id += 1 + expected_html += " |
{% trans %}Author: {% endtrans %}{{ user_profile_link(news.author) }}
{% if news.moderator %}{% trans %}Moderator: {% endtrans %}{{ user_profile_link(news.moderator) }}
- {% elif user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID) %} + {% elif user.is_com_admin %}{% trans %}Moderate{% endtrans %}
{% endif %} {% if user.can_edit(news) %} diff --git a/com/templates/com/news_edit.jinja b/com/templates/com/news_edit.jinja index 83c9a03b..dd86f544 100644 --- a/com/templates/com/news_edit.jinja +++ b/com/templates/com/news_edit.jinja @@ -49,7 +49,7 @@{{ form.club.errors }} {{ form.club }}
{{ form.summary.errors }} {{ form.summary }}
{{ form.content.errors }} {{ form.content }}
- {% if user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID) %} + {% if user.is_com_admin %}{{ form.automoderation.errors }} {{ form.automoderation }}
{% endif %} diff --git a/com/templates/com/news_list.jinja b/com/templates/com/news_list.jinja index 15eb7ea3..ba811403 100644 --- a/com/templates/com/news_list.jinja +++ b/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 %} diff --git a/com/tests.py b/com/tests.py index 29360dea..2a5206b2 100644 --- a/com/tests.py +++ b/com/tests.py @@ -13,16 +13,18 @@ # 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): @@ -110,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)) diff --git a/com/views.py b/com/views.py index 1acf1ef2..f8d0119b 100644 --- a/com/views.py +++ b/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) diff --git a/core/apps.py b/core/apps.py index 4f3358d5..cd131f57 100644 --- a/core/apps.py +++ b/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 diff --git a/core/management/commands/populate.py b/core/management/commands/populate.py index 8df2bfff..d89ff1e2 100644 --- a/core/management/commands/populate.py +++ b/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 diff --git a/core/models.py b/core/models.py index 376aaa3b..12f4e683 100644 --- a/core/models.py +++ b/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 @@ -880,13 +920,13 @@ class SithFile(models.Model): verbose_name = _("file") def is_owned_by(self, user): - if hasattr(self, "profile_of") and user.is_in_group( - settings.SITH_MAIN_BOARD_GROUP - ): + if user.is_anonymous: + return False + if hasattr(self, "profile_of") and user.is_board_member: return True - if user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID): + if user.is_com_admin: return True - if self.is_in_sas and user.is_in_group(settings.SITH_GROUP_SAS_ADMIN_ID): + 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 @@ -1493,6 +1533,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 diff --git a/core/signals.py b/core/signals.py new file mode 100644 index 00000000..7cda36c7 --- /dev/null +++ b/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") diff --git a/core/templates/core/file_detail.jinja b/core/templates/core/file_detail.jinja index 4db1f946..4d0a2887 100644 --- a/core/templates/core/file_detail.jinja +++ b/core/templates/core/file_detail.jinja @@ -61,7 +61,7 @@ {% if not file.home_of and not file.home_of_club and file.parent %} {% endif %} -{% if user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID) %} +{% if user.is_com_admin %} {% endif %} {% endblock %} diff --git a/core/templates/core/user_clubs.jinja b/core/templates/core/user_clubs.jinja index 5bdc91f3..f6e96b9a 100644 --- a/core/templates/core/user_clubs.jinja +++ b/core/templates/core/user_clubs.jinja @@ -67,7 +67,10 @@{{ sub.mailing.email }} {% trans %}Unsubscribe{% endtrans %}
diff --git a/core/templates/core/user_detail.jinja b/core/templates/core/user_detail.jinja index 5cc24de0..8a78ba48 100644 --- a/core/templates/core/user_detail.jinja +++ b/core/templates/core/user_detail.jinja @@ -136,7 +136,12 @@ - {% 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 #}{{ form["avatar_pict"].label }}
{{ 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 -%} {%- trans -%}Delete{%- endtrans -%} @@ -75,7 +75,7 @@{{ form["scrub_pict"].label }}
{{ 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 -%} {%- trans -%}Delete{%-endtrans -%} diff --git a/core/templates/core/user_tools.jinja b/core/templates/core/user_tools.jinja index 66e64d58..4e1e69a2 100644 --- a/core/templates/core/user_tools.jinja +++ b/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) %}
- {% if user.is_in_group(settings.SITH_GROUP_FORUM_ADMIN_ID) or user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID) or user.can_edit(forum) %}
+ {%
+ if user.is_com_admin
+ or user.is_in_group(pk=settings.SITH_GROUP_FORUM_ADMIN_ID)
+ or user.can_edit(forum)
+ %}
{% trans %}New forum{% endtrans %}
{% endif %}
{% if not forum.is_category %}
diff --git a/forum/templates/forum/main.jinja b/forum/templates/forum/main.jinja
index 8bedc310..e6a62785 100644
--- a/forum/templates/forum/main.jinja
+++ b/forum/templates/forum/main.jinja
@@ -17,7 +17,10 @@
{% trans %}Favorite topics{% endtrans %}
{{ display_search_bar(request) }}
{% trans %}New forum{% endtrans %}
diff --git a/launderette/models.py b/launderette/models.py index 7d8b2267..3ca12b2d 100644 --- a/launderette/models.py +++ b/launderette/models.py @@ -42,6 +42,8 @@ class Launderette(models.Model): """ Method to see if that object can be edited by the given user """ + if user.is_anonymous: + return False launderette_club = Club.objects.filter( unix_name=settings.SITH_LAUNDERETTE_MANAGER["unix_name"] ).first() @@ -60,7 +62,7 @@ class Launderette(models.Model): return False def can_be_viewed_by(self, user): - return user.is_in_group(settings.SITH_MAIN_MEMBERS_GROUP) + return user.is_subscribed def __str__(self): return self.name @@ -101,6 +103,8 @@ class Machine(models.Model): """ Method to see if that object can be edited by the given user """ + if user.is_anonymous: + return False launderette_club = Club.objects.filter( unix_name=settings.SITH_LAUNDERETTE_MANAGER["unix_name"] ).first() @@ -155,6 +159,8 @@ class Token(models.Model): """ Method to see if that object can be edited by the given user """ + if user.is_anonymous: + return False launderette_club = Club.objects.filter( unix_name=settings.SITH_LAUNDERETTE_MANAGER["unix_name"] ).first() diff --git a/launderette/templates/launderette/launderette_book_choose.jinja b/launderette/templates/launderette/launderette_book_choose.jinja index af30c6c1..d80505b4 100644 --- a/launderette/templates/launderette/launderette_book_choose.jinja +++ b/launderette/templates/launderette/launderette_book_choose.jinja @@ -5,7 +5,7 @@ {% endblock %} {% block content %} -{% if request.user.is_in_group(settings.SITH_MAIN_MEMBERS_GROUP) %} +{% if request.user.is_subscribed %}