diff --git a/backend/clubs/serializers.py b/backend/clubs/serializers.py index 865223174..4e09d13d3 100644 --- a/backend/clubs/serializers.py +++ b/backend/clubs/serializers.py @@ -994,30 +994,48 @@ def get_fields(self): def to_representation(self, instance): """ - Return the previous approved version of a club for users - that should not see unapproved content. + Return appropriate version of club based on user permissions and club status: + - Privileged users see current version (or when bypass=True) + - For non-privileged users: + - During renewal period: show last approved version from past calendar year + - Outside renewal period: show last approved version from this renewal cycle """ - if instance.ghost and not instance.approved: - user = self.context["request"].user + user = self.context["request"].user + can_see_pending = user.has_perm("clubs.see_pending_clubs") or user.has_perm( + "clubs.manage_club" + ) + is_member = ( + user.is_authenticated + and instance.membership_set.filter(person=user).exists() + ) + bypass = ( + self.context["request"].query_params.get("bypass", "").lower() == "true" + ) - can_see_pending = user.has_perm("clubs.see_pending_clubs") or user.has_perm( - "clubs.manage_club" - ) - is_member = ( - user.is_authenticated - and instance.membership_set.filter(person=user).exists() - ) - if not can_see_pending and not is_member: - historical_club = ( - instance.history.filter(approved=True) - .order_by("-approved_on") - .first() - ) - if historical_club is not None: - approved_instance = historical_club.instance - approved_instance._is_historical = True - return super().to_representation(approved_instance) - return super().to_representation(instance) + if can_see_pending or is_member or instance.approved or bypass: + return super().to_representation(instance) + + now = timezone.now() + renewal_start, renewal_end = settings.RENEWAL_PERIOD + in_renewal_period = renewal_start <= now <= renewal_end + + start_date = ( + renewal_start + if not in_renewal_period + else now.replace(year=now.year - 1, month=1, day=1) + ) + approved_version = ( + instance.history.filter(approved=True, approved_on__gte=start_date) + .order_by("-approved_on") + .first() + ) + + if approved_version is not None: + approved_instance = approved_version.instance + approved_instance._is_historical = True + return super().to_representation(approved_instance) + + raise serializers.ValidationError("Club not found", code="not_found") class Meta: model = Club diff --git a/backend/pennclubs/settings/base.py b/backend/pennclubs/settings/base.py index 4c645ea94..a235f47c8 100644 --- a/backend/pennclubs/settings/base.py +++ b/backend/pennclubs/settings/base.py @@ -13,6 +13,7 @@ import os import dj_database_url +from django.utils import timezone # Build paths inside the project like this: os.path.join(BASE_DIR, ...) @@ -204,11 +205,16 @@ OSA_EMAILS = ["vpul-orgs@pobox.upenn.edu"] - -# Controls whether existing clubs can submit for reapproval -REAPPROVAL_QUEUE_OPEN = True -# Controls whether new clubs can submit for initial approval -NEW_APPROVAL_QUEUE_OPEN = True +REAPPROVAL_QUEUE_OPEN = True # controls whether existing clubs can request reapproval +NEW_APPROVAL_QUEUE_OPEN = True # controls whether new clubs can request approval +RENEWAL_PERIOD = ( + timezone.datetime( + timezone.now().year, 8, 1, tzinfo=timezone.get_default_timezone() + ), + timezone.datetime( + timezone.now().year, 9, 30, tzinfo=timezone.get_default_timezone() + ), +) # defines renewal period for club visibility # File upload settings diff --git a/backend/tests/clubs/test_serializers.py b/backend/tests/clubs/test_serializers.py new file mode 100644 index 000000000..184bac344 --- /dev/null +++ b/backend/tests/clubs/test_serializers.py @@ -0,0 +1,152 @@ +from datetime import timedelta +from unittest import mock + +from django.conf import settings +from django.contrib.auth.models import Permission, User +from django.test import TestCase +from django.urls import reverse +from django.utils import timezone +from rest_framework.exceptions import ValidationError +from rest_framework.request import Request +from rest_framework.test import APIRequestFactory + +from clubs.models import Club, Membership +from clubs.serializers import ClubSerializer + + +class ClubSerializerTestCase(TestCase): + def setUp(self): + self.factory = APIRequestFactory() + self.user = User.objects.create_user(username="testuser", password="12345") + self.club = Club.objects.create(name="Test Club", code="test", approved=True) + + # Set renewal period in settings to current time + now = timezone.now() + self.renewal_start = now - timedelta(days=30) + self.renewal_end = now + timedelta(days=30) + settings.RENEWAL_PERIOD = (self.renewal_start, self.renewal_end) + + self.serializer = ClubSerializer() + + def test_privileged_user_sees_current_version(self): + self.user.user_permissions.add( + Permission.objects.get(codename="see_pending_clubs") + ) + request = self.factory.get(reverse("clubs-detail", args=[self.club.code])) + request.user = self.user + + self.serializer.context["request"] = Request(request) + result = self.serializer.to_representation(self.club) + self.assertEqual(result["name"], "Test Club") + + def test_member_sees_current_version(self): + Membership.objects.create(person=self.user, club=self.club) + request = self.factory.get(reverse("clubs-detail", args=[self.club.code])) + request.user = self.user + + self.serializer.context["request"] = Request(request) + result = self.serializer.to_representation(self.club) + self.assertEqual(result["name"], "Test Club") + + def test_approved_club_visible_to_all(self): + request = self.factory.get(reverse("clubs-detail", args=[self.club.code])) + request.user = self.user + + self.serializer.context["request"] = Request(request) + result = self.serializer.to_representation(self.club) + self.assertEqual(result["name"], "Test Club") + + def test_bypass_flag_shows_current_version(self): + request = self.factory.get( + f"{reverse('clubs-detail', args=[self.club.code])}?bypass=true" + ) + request.user = self.user + + self.serializer.context["request"] = Request(request) + result = self.serializer.to_representation(self.club) + self.assertEqual(result["name"], "Test Club") + + def test_non_privileged_user_during_renewal_period(self): + # Simulate deactivation at the beginning of the renewal period + self.club.save() + self.club.approved = False + self.club.save() + + # Insert a previously approved version into the club's history + past_time = timezone.now() - timedelta(days=180) + with mock.patch("django.utils.timezone.now", return_value=past_time): + self.club.name = "Past Approved Version" + self.club.approved = True + self.club.approved_on = past_time + self.club.save() + + self.club.name = "Current Unapproved Version" + self.club.approved = False + self.club.approved_on = None + self.club.save() + + request = self.factory.get("/fake-url/") + request.user = self.user + serializer = ClubSerializer(context={"request": Request(request)}) + + result = serializer.to_representation(self.club) + self.assertEqual(result["name"], "Past Approved Version") + self.assertNotEqual(result["name"], "Current Unapproved Version") + + def test_non_privileged_user_outside_renewal_period_ghost_club(self): + self.club.approved = False + self.club.ghost = True + self.club.save() + + current_date = self.renewal_end + timedelta(days=1) + with mock.patch("django.utils.timezone.now", return_value=current_date): + approved_date = self.renewal_start + timedelta(days=1) + with mock.patch("django.utils.timezone.now", return_value=approved_date): + self.club.name = "Current Cycle Approved Version" + self.club.approved = True + self.club.approved_on = approved_date + self.club.save() + + self.club.name = "Current Unapproved Version" + self.club.approved = False + self.club.approved_on = None + self.club.save() + + request = self.factory.get(reverse("clubs-detail", args=[self.club.code])) + request.user = self.user + + self.serializer.context["request"] = Request(request) + result = self.serializer.to_representation(self.club) + self.assertEqual(result["name"], "Current Cycle Approved Version") + self.assertNotEqual(result["name"], "Current Unapproved Version") + + def test_non_privileged_user_no_approved_version(self): + self.club.approved = False + self.club.save() + + request = self.factory.get(reverse("clubs-detail", args=[self.club.code])) + request.user = self.user + self.serializer.context["request"] = Request(request) + + with self.assertRaises(ValidationError) as context: + self.serializer.to_representation(self.club) + + self.assertEqual(context.exception.detail[0], "Club not found") + self.assertEqual(context.exception.get_codes()[0], "not_found") + + def test_non_privileged_user_outside_renewal_period_non_ghost_club(self): + self.club.approved = False + self.club.ghost = False + self.club.save() + + current_date = self.renewal_end + timedelta(days=1) + with mock.patch("django.utils.timezone.now", return_value=current_date): + request = self.factory.get(reverse("clubs-detail", args=[self.club.code])) + request.user = self.user + + self.serializer.context["request"] = Request(request) + with self.assertRaises(ValidationError) as context: + self.serializer.to_representation(self.club) + + self.assertEqual(context.exception.detail[0], "Club not found") + self.assertEqual(context.exception.get_codes()[0], "not_found") # diff --git a/backend/tests/clubs/test_views.py b/backend/tests/clubs/test_views.py index 964269930..0d36de20c 100644 --- a/backend/tests/clubs/test_views.py +++ b/backend/tests/clubs/test_views.py @@ -175,8 +175,9 @@ def setUp(self): self.club1 = Club.objects.create( code="test-club", name="Test Club", - approved=True, email="example@example.com", + approved=True, + approved_on=timezone.now(), ) self.event1 = Event.objects.create(