From 294b59b4d6abbf6b74b45f1f41e4917eb80f2d06 Mon Sep 17 00:00:00 2001 From: imperosol Date: Tue, 11 Feb 2025 14:24:24 +0100 Subject: [PATCH] use django auth for subscription creation page --- core/management/commands/populate.py | 5 ++++ core/models.py | 31 --------------------- core/templates/core/user_detail.jinja | 2 +- core/templates/core/user_tools.jinja | 4 +-- sith/settings.py | 8 ------ subscription/tests/test_new_susbcription.py | 15 ++++++++-- subscription/views.py | 16 +++++------ 7 files changed, 28 insertions(+), 53 deletions(-) diff --git a/core/management/commands/populate.py b/core/management/commands/populate.py index 26bc6074..53638699 100644 --- a/core/management/commands/populate.py +++ b/core/management/commands/populate.py @@ -125,6 +125,11 @@ class Command(BaseCommand): unix_name=settings.SITH_MAIN_CLUB["unix_name"], address=settings.SITH_MAIN_CLUB["address"], ) + main_club.board_group.permissions.add( + *Permission.objects.filter( + codename__in=["view_subscription", "add_subscription"] + ) + ) bar_club = Club.objects.create( id=2, name=settings.SITH_BAR_MANAGER["name"], diff --git a/core/models.py b/core/models.py index b1caa912..4748f311 100644 --- a/core/models.py +++ b/core/models.py @@ -417,29 +417,6 @@ class User(AbstractUser): def is_board_member(self) -> bool: return self.groups.filter(club_board=settings.SITH_MAIN_CLUB_ID).exists() - @cached_property - def can_read_subscription_history(self) -> bool: - if self.is_root or self.is_board_member: - return True - - from club.models import Club - - for club in Club.objects.filter( - id__in=settings.SITH_CAN_READ_SUBSCRIPTION_HISTORY - ): - if club in self.clubs_with_rights: - return True - return False - - @cached_property - def can_create_subscription(self) -> bool: - return self.is_root or ( - self.memberships.board() - .ongoing() - .filter(club_id__in=settings.SITH_CAN_CREATE_SUBSCRIPTIONS) - .exists() - ) - @cached_property def is_launderette_manager(self): from club.models import Club @@ -679,14 +656,6 @@ class AnonymousUser(AuthAnonymousUser): def __init__(self): super().__init__() - @property - def can_create_subscription(self): - return False - - @property - def can_read_subscription_history(self): - return False - @property def was_subscribed(self): return False diff --git a/core/templates/core/user_detail.jinja b/core/templates/core/user_detail.jinja index cb93b1cd..22db9abe 100644 --- a/core/templates/core/user_detail.jinja +++ b/core/templates/core/user_detail.jinja @@ -166,7 +166,7 @@ {% endif %}
-{% if profile.was_subscribed and (user == profile or user.can_read_subscription_history)%} +{% if profile.was_subscribed and (user == profile or user.has_perm("subscription.view_subscription")) %}
diff --git a/core/templates/core/user_tools.jinja b/core/templates/core/user_tools.jinja index 177b4199..bafefe3c 100644 --- a/core/templates/core/user_tools.jinja +++ b/core/templates/core/user_tools.jinja @@ -13,7 +13,7 @@

{% trans %}User Tools{% endtrans %}

- {% if user.can_create_subscription or user.is_root or user.is_board_member %} + {% if user.has_perm("subscription.add_subscription") or user.is_root or user.is_board_member %}

{% trans %}Sith management{% endtrans %}

    @@ -26,7 +26,7 @@ {% if user.has_perm("core.view_userban") %}
  • {% trans %}Bans{% endtrans %}
    • {% endif %} - {% if user.can_create_subscription or user.is_root %} + {% if user.has_perm("subscription.add_subscription") %}
  • {% trans %}Subscriptions{% endtrans %}
    • {% endif %} {% if user.is_board_member or user.is_root %} diff --git a/sith/settings.py b/sith/settings.py index d3acabec..b7d7e71c 100644 --- a/sith/settings.py +++ b/sith/settings.py @@ -517,14 +517,6 @@ SITH_PRODUCT_SUBSCRIPTION_ONE_SEMESTER = 1 SITH_PRODUCT_SUBSCRIPTION_TWO_SEMESTERS = 2 SITH_PRODUCTTYPE_SUBSCRIPTION = 2 -# Defines which club lets its member the ability to make subscriptions -# Elements of this list are club's id -SITH_CAN_CREATE_SUBSCRIPTIONS = [1] - -# Defines which clubs lets its members the ability to see users subscription history -# Elements of this list are club's id -SITH_CAN_READ_SUBSCRIPTION_HISTORY = [] - # Number of weeks before the end of a subscription when the subscriber can resubscribe SITH_SUBSCRIPTION_END = 10 diff --git a/subscription/tests/test_new_susbcription.py b/subscription/tests/test_new_susbcription.py index ccdff407..8fd5e7c4 100644 --- a/subscription/tests/test_new_susbcription.py +++ b/subscription/tests/test_new_susbcription.py @@ -5,6 +5,7 @@ from typing import Callable import pytest from dateutil.relativedelta import relativedelta +from django.contrib.auth.models import Permission from django.test import Client from django.urls import reverse from django.utils.timezone import localdate @@ -108,7 +109,12 @@ def test_page_access( @pytest.mark.django_db def test_submit_form_existing_user(client: Client, settings: SettingsWrapper): - client.force_login(board_user.make()) + client.force_login( + baker.make( + User, + user_permissions=Permission.objects.filter(codename="add_subscription"), + ) + ) user = old_subscriber_user.make() response = client.post( reverse("subscription:fragment-existing-user"), @@ -133,7 +139,12 @@ def test_submit_form_existing_user(client: Client, settings: SettingsWrapper): @pytest.mark.django_db def test_submit_form_new_user(client: Client, settings: SettingsWrapper): - client.force_login(board_user.make()) + client.force_login( + baker.make( + User, + user_permissions=Permission.objects.filter(codename="add_subscription"), + ) + ) response = client.post( reverse("subscription:fragment-new-user"), { diff --git a/subscription/views.py b/subscription/views.py index b285a137..ce1ad4d7 100644 --- a/subscription/views.py +++ b/subscription/views.py @@ -14,7 +14,7 @@ # from django.conf import settings -from django.contrib.auth.mixins import UserPassesTestMixin +from django.contrib.auth.mixins import PermissionRequiredMixin from django.core.exceptions import PermissionDenied from django.urls import reverse, reverse_lazy from django.utils.timezone import localdate @@ -30,13 +30,9 @@ from subscription.forms import ( from subscription.models import Subscription -class CanCreateSubscriptionMixin(UserPassesTestMixin): - def test_func(self): - return self.request.user.can_create_subscription - - -class NewSubscription(CanCreateSubscriptionMixin, TemplateView): +class NewSubscription(PermissionRequiredMixin, TemplateView): template_name = "subscription/subscription.jinja" + permission_required = "subscription.add_subscription" def get_context_data(self, **kwargs): return super().get_context_data(**kwargs) | { @@ -49,8 +45,9 @@ class NewSubscription(CanCreateSubscriptionMixin, TemplateView): } -class CreateSubscriptionFragment(CanCreateSubscriptionMixin, CreateView): +class CreateSubscriptionFragment(PermissionRequiredMixin, CreateView): template_name = "subscription/fragments/creation_form.jinja" + permission_required = "subscription.add_subscription" def get_success_url(self): return reverse( @@ -72,8 +69,9 @@ class CreateSubscriptionNewUserFragment(CreateSubscriptionFragment): extra_context = {"post_url": reverse_lazy("subscription:fragment-new-user")} -class SubscriptionCreatedFragment(CanCreateSubscriptionMixin, DetailView): +class SubscriptionCreatedFragment(PermissionRequiredMixin, DetailView): template_name = "subscription/fragments/creation_success.jinja" + permission_required = "subscription.add_subscription" model = Subscription pk_url_kwarg = "subscription_id" context_object_name = "subscription"