From 308fc0b133dc1878a8496c40410ce00b936996a7 Mon Sep 17 00:00:00 2001 From: Avi Upadhyayula <69180850+aviupadhyayula@users.noreply.github.com> Date: Fri, 27 Sep 2024 19:29:26 -0400 Subject: [PATCH 01/45] Use Club.get_officer_emails() to send alerts (#729) --- backend/clubs/models.py | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/backend/clubs/models.py b/backend/clubs/models.py index d75e1ab02..bb95fcedc 100644 --- a/backend/clubs/models.py +++ b/backend/clubs/models.py @@ -782,11 +782,7 @@ def __str__(self): def send_question_mail(self, request=None): domain = get_domain(request) - owner_emails = list( - self.club.membership_set.filter( - role__lte=Membership.ROLE_OFFICER, active=True - ).values_list("person__email", flat=True) - ) + emails = self.club.get_officer_emails() context = { "name": self.club.name, @@ -794,11 +790,11 @@ def send_question_mail(self, request=None): "url": settings.QUESTION_URL.format(domain=domain, club=self.club.code), } - if owner_emails: + if emails: send_mail_helper( name="question", subject="Question for {}".format(self.club.name), - emails=owner_emails, + emails=emails, context=context, ) @@ -1114,20 +1110,17 @@ def send_request(self, request=None): "full_name": self.person.get_full_name(), } - owner_emails = list( - self.club.membership_set.filter( - role__lte=Membership.ROLE_OFFICER, active=True - ).values_list("person__email", flat=True) - ) + emails = self.club.get_officer_emails() - send_mail_helper( - name="request", - subject="Membership Request from {} for {}".format( - self.person.get_full_name(), self.club.name - ), - emails=owner_emails, - context=context, - ) + if emails: + send_mail_helper( + name="request", + subject="Membership Request from {} for {}".format( + self.person.get_full_name(), self.club.name + ), + emails=emails, + context=context, + ) class Meta: unique_together = (("person", "club"),) From 50c89dbbaf418a381735083111aea48a7302a7ca Mon Sep 17 00:00:00 2001 From: aviupadhyayula Date: Fri, 27 Sep 2024 19:59:58 -0400 Subject: [PATCH 02/45] Refactor get_officer_emails --- backend/clubs/models.py | 36 +++++++++----------- backend/tests/clubs/test_models.py | 54 ++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 20 deletions(-) diff --git a/backend/clubs/models.py b/backend/clubs/models.py index bb95fcedc..6a3c804a0 100644 --- a/backend/clubs/models.py +++ b/backend/clubs/models.py @@ -620,27 +620,23 @@ def get_officer_emails(self): """ emails = [] - # add club contact email if valid - try: - validate_email(self.email) - emails.append(self.email) - except ValidationError: - pass - - # add email for all officers and above - for user in self.membership_set.filter( - role__lte=Membership.ROLE_OFFICER, active=True - ): - emails.append(user.person.email) - - # remove empty emails - emails = [email.strip() for email in emails] - emails = [email for email in emails if email] - - # remove duplicate emails - emails = list(sorted(set(emails))) + # Add club contact email if valid + if self.email: + try: + validate_email(self.email) + emails.append(self.email) + except ValidationError: + pass + + # Add email for all active officers and above + emails.extend( + self.membership_set.filter( + role__lte=Membership.ROLE_OFFICER, active=True + ).values_list("person__email", flat=True) + ) - return emails + # Remove whitespace, empty emails, and duplicates, then sort + return sorted(set(email.strip() for email in emails if email.strip())) def send_confirmation_email(self, request=None): """ diff --git a/backend/tests/clubs/test_models.py b/backend/tests/clubs/test_models.py index e70ca7fdf..477be3fcb 100644 --- a/backend/tests/clubs/test_models.py +++ b/backend/tests/clubs/test_models.py @@ -63,6 +63,60 @@ def test_parent_children(self): self.assertEqual(self.club2.parent_orgs.first(), self.club1) self.assertEqual(self.club1.children_orgs.first(), self.club2) + def test_get_officer_emails(self): + # Create test users with various email formats + user1 = get_user_model().objects.create_user( + "user1", "user1@example.com", "password" + ) + user2 = get_user_model().objects.create_user( + "user2", " user2@example.com ", "password" + ) # whitespace + user3 = get_user_model().objects.create_user( + "user3", "", "password" + ) # empty email + user4 = get_user_model().objects.create_user( + "user4", "user4@example.com", "password" + ) + + # Create memberships for the test users + Membership.objects.create( + person=user1, club=self.club1, role=Membership.ROLE_OFFICER + ) + Membership.objects.create( + person=user2, club=self.club1, role=Membership.ROLE_OWNER + ) + Membership.objects.create( + person=user3, club=self.club1, role=Membership.ROLE_OFFICER + ) + Membership.objects.create( + person=user4, club=self.club1, role=Membership.ROLE_OFFICER, active=False + ) # alumni + + # Test with valid club email + self.club1.email = "club@example.com" + self.club1.save() + + officer_emails = self.club1.get_officer_emails() + expected_emails = ["club@example.com", "user1@example.com", "user2@example.com"] + self.assertEqual(officer_emails, expected_emails) + + # Ensure alumni are not included + self.assertNotIn("user4@example.com", officer_emails) + + # Test with invalid club email + self.club1.email = "invalid-email" + self.club1.save() + + officer_emails = self.club1.get_officer_emails() + expected_emails = ["user1@example.com", "user2@example.com"] + self.assertEqual(officer_emails, expected_emails) + + # Test with empty club email + self.club1.email = "" + self.club1.save() + officer_emails = self.club1.get_officer_emails() + self.assertEqual(officer_emails, expected_emails) + class ProfileTestCase(TestCase): def test_profile_creation(self): From 9612017059ad3805491b1622d2ec197a3043be0d Mon Sep 17 00:00:00 2001 From: Rohan Moniz <60864468+rm03@users.noreply.github.com> Date: Fri, 27 Sep 2024 20:30:54 -0400 Subject: [PATCH 03/45] add back cronjob for expiring stale membership invites --- k8s/main.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/k8s/main.ts b/k8s/main.ts index 98f01c587..086779086 100644 --- a/k8s/main.ts +++ b/k8s/main.ts @@ -88,6 +88,13 @@ export class MyChart extends PennLabsChart { secret: clubsSecret, cmd: ['python', 'manage.py', 'import_calendar_events'], }); + + new CronJob(this, 'expire-stale-membership-invites', { + schedule: cronTime.everyDayAt(12), + image: backendImage, + secret: clubsSecret, + cmd: ["python", "manage.py", "expire_membership_invites"], + }); } } From d3c7c9cd1318a59b43fd1f64917bc90ba919442a Mon Sep 17 00:00:00 2001 From: Rohan Moniz <60864468+rm03@users.noreply.github.com> Date: Mon, 30 Sep 2024 15:36:48 -0400 Subject: [PATCH 04/45] Move graduate_users script to cronjob (#730) * move grad_users script to cronjob * Add tests * Change cronjob schedule * Revert "Change cronjob schedule" This reverts commit d3cfc81e999788a490b2609bed52f37a9ead7187. --------- Co-authored-by: aviupadhyayula --- .../management/commands/graduate_users.py | 1 - backend/tests/clubs/test_commands.py | 44 +++++++++++++++++++ backend/tests/clubs/test_views.py | 2 +- k8s/main.ts | 7 +++ 4 files changed, 52 insertions(+), 2 deletions(-) diff --git a/backend/clubs/management/commands/graduate_users.py b/backend/clubs/management/commands/graduate_users.py index bf61ca50d..d6e000f63 100644 --- a/backend/clubs/management/commands/graduate_users.py +++ b/backend/clubs/management/commands/graduate_users.py @@ -9,7 +9,6 @@ class Command(BaseCommand): "Mark all memberships where the student has graduated as inactive. " "This script should be run at the beginning of each year." ) - web_execute = True def handle(self, *args, **kwargs): now = timezone.now() diff --git a/backend/tests/clubs/test_commands.py b/backend/tests/clubs/test_commands.py index 32be3469c..da5bafc71 100644 --- a/backend/tests/clubs/test_commands.py +++ b/backend/tests/clubs/test_commands.py @@ -733,3 +733,47 @@ def test_expire_membership_invites(self): self.assertFalse(self.expired_invite.active) self.assertTrue(self.active_invite.active) + + +class GraduateUsersTestCase(TestCase): + def setUp(self): + self.club = Club.objects.create(code="test", name="Test Club", active=True) + self.user1 = get_user_model().objects.create_user( + "bfranklin", "bfranklin@seas.upenn.edu", "test" + ) + self.user2 = get_user_model().objects.create_user( + "tjefferson", "tjefferson@seas.upenn.edu", "test" + ) + + # Set graduation years + self.user1.profile.graduation_year = timezone.now().year - 1 + self.user1.profile.save() + self.user2.profile.graduation_year = timezone.now().year + 1 + self.user2.profile.save() + + # Create active memberships + Membership.objects.create(person=self.user1, club=self.club, active=True) + Membership.objects.create(person=self.user2, club=self.club, active=True) + + def test_graduate_users(self): + # Ensure both memberships are active initially + self.assertEqual(Membership.objects.filter(active=True).count(), 2) + + # Run the command + call_command("graduate_users") + + # Check that only the graduated user's membership is inactive + self.assertEqual(Membership.objects.filter(active=True).count(), 1) + self.assertFalse(Membership.objects.get(person=self.user1).active) + self.assertTrue(Membership.objects.get(person=self.user2).active) + + def test_graduate_users_output(self): + # Capture command output + out = io.StringIO() + call_command("graduate_users", stdout=out) + + # Check the output + self.assertIn( + "Updated the membership status of 1 student club relationships!", + out.getvalue(), + ) diff --git a/backend/tests/clubs/test_views.py b/backend/tests/clubs/test_views.py index fe55fa5ba..564bcdc70 100644 --- a/backend/tests/clubs/test_views.py +++ b/backend/tests/clubs/test_views.py @@ -2600,7 +2600,7 @@ def test_execute_script(self): self.assertIn(resp.status_code, [200], resp.content) self.assertIsInstance(resp.data, list, resp.content) - resp = self.client.post(reverse("scripts"), {"action": "graduate_users"}) + resp = self.client.post(reverse("scripts"), {"action": "find_broken_images"}) self.assertIn(resp.status_code, [200], resp.content) self.assertIn("output", resp.data, resp.content) diff --git a/k8s/main.ts b/k8s/main.ts index 086779086..1a592a5f7 100644 --- a/k8s/main.ts +++ b/k8s/main.ts @@ -95,6 +95,13 @@ export class MyChart extends PennLabsChart { secret: clubsSecret, cmd: ["python", "manage.py", "expire_membership_invites"], }); + + new CronJob(this, 'graduate-users', { + schedule: cronTime.everyYearIn(1, 1, 12, 0), + image: backendImage, + secret: clubsSecret, + cmd: ["python", "manage.py", "graduate_users"], + }); } } From 245e557d0f634a39ba6cd1c61e54314159a8e514 Mon Sep 17 00:00:00 2001 From: Julian Weng Date: Wed, 2 Oct 2024 23:03:09 -0400 Subject: [PATCH 05/45] Hide personal information for non-authenticated users on club page (#732) Add granular visibility per advisor (admin, students, public) and hide question respondent names for unauthed view --- backend/clubs/admin.py | 2 +- .../clubs/management/commands/deactivate.py | 3 +- backend/clubs/management/commands/populate.py | 2 +- .../migrations/0114_alter_advisor_public.py | 35 +++++++++ backend/clubs/models.py | 15 +++- backend/clubs/serializers.py | 10 ++- backend/clubs/views.py | 32 ++++---- backend/tests/clubs/test_commands.py | 2 +- backend/tests/clubs/test_models.py | 5 +- backend/tests/clubs/test_views.py | 72 +++++++++++++++++ .../components/ClubEditPage/AdvisorCard.tsx | 77 ++++++++----------- frontend/components/ClubPage/QuestionList.tsx | 1 - frontend/types.ts | 8 +- 13 files changed, 195 insertions(+), 69 deletions(-) create mode 100644 backend/clubs/migrations/0114_alter_advisor_public.py diff --git a/backend/clubs/admin.py b/backend/clubs/admin.py index f0f3b73ef..e48ba1678 100644 --- a/backend/clubs/admin.py +++ b/backend/clubs/admin.py @@ -324,7 +324,7 @@ def club(self, obj): class AdvisorAdmin(admin.ModelAdmin): search_fields = ("name", "title", "email", "phone", "club__name") - list_display = ("name", "title", "email", "phone", "club", "public") + list_display = ("name", "title", "email", "phone", "club", "visibility") def club(self, obj): return obj.club.name diff --git a/backend/clubs/management/commands/deactivate.py b/backend/clubs/management/commands/deactivate.py index a2488c3c6..f8eb0f427 100644 --- a/backend/clubs/management/commands/deactivate.py +++ b/backend/clubs/management/commands/deactivate.py @@ -93,7 +93,8 @@ def handle(self, *args, **kwargs): num_ghosted += 1 club.save() - cache.delete(f"clubs:{club.id}") # clear cache + cache.delete(f"clubs:{club.id}-authed") # clear cache + cache.delete(f"clubs:{club.id}-anon") self.stdout.write( f"{clubs.count()} clubs deactivated! {num_ghosted} clubs ghosted!" diff --git a/backend/clubs/management/commands/populate.py b/backend/clubs/management/commands/populate.py index 892865ce5..310b17804 100644 --- a/backend/clubs/management/commands/populate.py +++ b/backend/clubs/management/commands/populate.py @@ -455,7 +455,7 @@ def get_image(url): department="Accounting Department", email="example@example.com", phone="+12158985000", - defaults={"public": True}, + defaults={"visibility": Advisor.ADVISOR_VISIBILITY_STUDENTS}, ) club.tags.add(tag_undergrad) diff --git a/backend/clubs/migrations/0114_alter_advisor_public.py b/backend/clubs/migrations/0114_alter_advisor_public.py new file mode 100644 index 000000000..3ab692de9 --- /dev/null +++ b/backend/clubs/migrations/0114_alter_advisor_public.py @@ -0,0 +1,35 @@ +# Generated by Django 5.0.4 on 2024-09-29 17:00 + +from django.db import migrations, models + + +def migrate_public_to_enum(apps, schema_editor): + Advisor = apps.get_model("clubs", "Advisor") + # Update 'public' field, assume public=True means only students by default + Advisor.objects.filter(public=False).update(visibility=1) + Advisor.objects.filter(public=True).update(visibility=2) + +def reverse_migrate_public_to_enum(apps, schema_editor): + Advisor = apps.get_model("clubs", "Advisor") + Advisor.objects.filter(visibility=1).update(public=False) + Advisor.objects.filter(visibility__in=[2, 3]).update(public=True) + +class Migration(migrations.Migration): + dependencies = [ + ("clubs", "0113_badge_message"), + ] + operations = [ + migrations.AddField( + model_name="advisor", + name="visibility", + field=models.IntegerField( + choices=[(1, "Admin Only"), (2, "Signed-in Students"), (3, "Public")], + default=2, + ), + ), + migrations.RunPython(migrate_public_to_enum, reverse_migrate_public_to_enum), + migrations.RemoveField( + model_name="advisor", + name="public", + ), + ] diff --git a/backend/clubs/models.py b/backend/clubs/models.py index 6a3c804a0..36cc23044 100644 --- a/backend/clubs/models.py +++ b/backend/clubs/models.py @@ -1136,7 +1136,20 @@ class Advisor(models.Model): phone = PhoneNumberField(null=False, blank=True) club = models.ForeignKey(Club, on_delete=models.CASCADE) - public = models.BooleanField() + + ADVISOR_VISIBILITY_ADMIN = 1 + ADVISOR_VISIBILITY_STUDENTS = 2 + ADVISOR_VISIBILITY_ALL = 3 + + ADVISOR_VISIBILITY_CHOICES = ( + (ADVISOR_VISIBILITY_ADMIN, "Admin Only"), + (ADVISOR_VISIBILITY_STUDENTS, "Signed-in Students"), + (ADVISOR_VISIBILITY_ALL, "Public"), + ) + + visibility = models.IntegerField( + choices=ADVISOR_VISIBILITY_CHOICES, default=ADVISOR_VISIBILITY_STUDENTS + ) def __str__(self): return self.name diff --git a/backend/clubs/serializers.py b/backend/clubs/serializers.py index 492ca171d..865223174 100644 --- a/backend/clubs/serializers.py +++ b/backend/clubs/serializers.py @@ -351,7 +351,7 @@ class AdvisorSerializer( ): class Meta: model = Advisor - fields = ("id", "name", "title", "department", "email", "phone", "public") + fields = ("id", "name", "title", "department", "email", "phone", "visibility") class ClubEventSerializer(serializers.ModelSerializer): @@ -1249,8 +1249,14 @@ def get_approved_by(self, obj): return obj.approved_by.get_full_name() def get_advisor_set(self, obj): + user = self.context["request"].user + visibility_level = ( + Advisor.ADVISOR_VISIBILITY_STUDENTS + if user.is_authenticated + else Advisor.ADVISOR_VISIBILITY_ALL + ) return AdvisorSerializer( - obj.advisor_set.filter(public=True).order_by("name"), + obj.advisor_set.filter(visibility__gte=visibility_level), many=True, read_only=True, context=self.context, diff --git a/backend/clubs/views.py b/backend/clubs/views.py index bce5c749d..199e59d04 100644 --- a/backend/clubs/views.py +++ b/backend/clubs/views.py @@ -1204,8 +1204,8 @@ def upload(self, request, *args, **kwargs): """ # ensure user is allowed to upload image club = self.get_object() - key = f"clubs:{club.id}" - cache.delete(key) + cache.delete(f"clubs:{club.id}-authed") + cache.delete(f"clubs:{club.id}-anon") # reset approval status after upload resp = upload_endpoint_helper(request, club, "file", "image", save=False) @@ -2024,7 +2024,8 @@ def retrieve(self, *args, **kwargs): if self._has_elevated_view_perms(club): return super().retrieve(*args, **kwargs) - key = f"clubs:{club.id}" + key = f"""clubs:{club.id}-{"authed" if self.request.user.is_authenticated + else "anon"}""" cached = cache.get(key) if cached: return Response(cached) @@ -2038,8 +2039,8 @@ def update(self, request, *args, **kwargs): Invalidate caches """ self.check_approval_permission(request) - key = f"clubs:{self.get_object().id}" - cache.delete(key) + cache.delete(f"clubs:{self.get_object().id}-authed") + cache.delete(f"clubs:{self.get_object().id}-anon") return super().update(request, *args, **kwargs) def partial_update(self, request, *args, **kwargs): @@ -2047,8 +2048,8 @@ def partial_update(self, request, *args, **kwargs): Invalidate caches """ self.check_approval_permission(request) - key = f"clubs:{self.get_object().id}" - cache.delete(key) + cache.delete(f"clubs:{self.get_object().id}-authed") + cache.delete(f"clubs:{self.get_object().id}-anon") return super().partial_update(request, *args, **kwargs) def perform_destroy(self, instance): @@ -2279,11 +2280,14 @@ class AdvisorSearchFilter(filters.BaseFilterBackend): def filter_queryset(self, request, queryset, view): public = request.GET.get("public") - if public is not None: public = public.strip().lower() if public in {"true", "false"}: - queryset = queryset.filter(public=public == "true") + queryset = queryset.filter( + visibility__gte=Advisor.ADVISOR_VISIBILITY_STUDENTS + if public == "true" + else Advisor.ADVISOR_VISIBILITY_ADMIN + ) return queryset @@ -2291,7 +2295,7 @@ def filter_queryset(self, request, queryset, view): class AdvisorViewSet(viewsets.ModelViewSet): """ list: - Return a list of advisors for this club. + Return a list of advisors for this club for club administrators. create: Add an advisor to this club. @@ -2310,7 +2314,7 @@ class AdvisorViewSet(viewsets.ModelViewSet): """ serializer_class = AdvisorSerializer - permission_classes = [ClubItemPermission | IsSuperuser] + permission_classes = [ClubSensitiveItemPermission | IsSuperuser] filter_backends = [AdvisorSearchFilter] lookup_field = "id" http_method_names = ["get", "post", "put", "patch", "delete"] @@ -2818,7 +2822,8 @@ def create_tickets(self, request, *args, **kwargs): event.ticket_drop_time = drop_time event.save() - cache.delete(f"clubs:{event.club.id}") + cache.delete(f"clubs:{event.club.id}-authed") + cache.delete(f"clubs:{event.club.id}-anon") return Response({"detail": "Successfully created tickets"}) @action(detail=True, methods=["post"]) @@ -3422,7 +3427,8 @@ def get_queryset(self): ) if not self.request.user.is_authenticated: - return questions.filter(approved=True) + # Hide responder if not authenticated + return questions.filter(approved=True).extra(select={"responder": "NULL"}) membership = Membership.objects.filter( club__code=club_code, person=self.request.user diff --git a/backend/tests/clubs/test_commands.py b/backend/tests/clubs/test_commands.py index da5bafc71..48e3799b3 100644 --- a/backend/tests/clubs/test_commands.py +++ b/backend/tests/clubs/test_commands.py @@ -616,7 +616,7 @@ def test_deactivate_invalidates_cache(self): self.client.get(reverse("clubs-detail", args=(club.code,))) # club should now be cached - cache_key = f"clubs:{club.id}" + cache_key = f"clubs:{club.id}-anon" self.assertIsNotNone(caches["default"].get(cache_key)) call_command("deactivate", "all", "--force") diff --git a/backend/tests/clubs/test_models.py b/backend/tests/clubs/test_models.py index 477be3fcb..04e42d1d1 100644 --- a/backend/tests/clubs/test_models.py +++ b/backend/tests/clubs/test_models.py @@ -198,7 +198,10 @@ def setUp(self): code="a", name="a", subtitle="a", founded=date, description="a", size=1 ) self.advisor = Advisor.objects.create( - name="Eric Wang", phone="+12025550133", club=club, public=True + name="Eric Wang", + phone="+12025550133", + club=club, + visibility=Advisor.ADVISOR_VISIBILITY_ALL, ) def test_str(self): diff --git a/backend/tests/clubs/test_views.py b/backend/tests/clubs/test_views.py index 564bcdc70..964269930 100644 --- a/backend/tests/clubs/test_views.py +++ b/backend/tests/clubs/test_views.py @@ -18,6 +18,7 @@ from clubs.filters import DEFAULT_PAGE_SIZE from clubs.models import ( + Advisor, ApplicationSubmission, Asset, Badge, @@ -204,6 +205,77 @@ def setUp(self): responder=self.user1, ) + self.advisor_admin = Advisor.objects.create( + name="Anonymous Avi", + phone="+12025550133", + club=self.club1, + visibility=Advisor.ADVISOR_VISIBILITY_ADMIN, + ) + + self.advisor_students = Advisor.objects.create( + name="Reclusive Rohan", + phone="+12025550133", + club=self.club1, + visibility=Advisor.ADVISOR_VISIBILITY_STUDENTS, + ) + + self.advisor_public = Advisor.objects.create( + name="Jocular Julian", + phone="+12025550133", + club=self.club1, + visibility=Advisor.ADVISOR_VISIBILITY_ALL, + ) + + def test_advisor_visibility(self): + """ + Tests each tier of advisor visibility. + """ + # Anonymous view + self.client.logout() + resp = self.client.get(reverse("clubs-detail", args=(self.club1.code,))) + self.assertIn(resp.status_code, [200, 201], resp.content) + self.assertEqual(len(resp.data["advisor_set"]), 1) + self.assertEqual(resp.data["advisor_set"][0]["name"], "Jocular Julian") + + # Student view + self.client.login(username=self.user1.username, password="test") + resp = self.client.get(reverse("clubs-detail", args=(self.club1.code,))) + self.assertIn(resp.status_code, [200, 201], resp.content) + self.assertEqual(len(resp.data["advisor_set"]), 2) + sorted_advisors = sorted( + [advisor["name"] for advisor in resp.data["advisor_set"]] + ) + self.assertEqual(sorted_advisors, ["Jocular Julian", "Reclusive Rohan"]) + + # Admin view + self.client.login(username=self.user5.username, password="test") + resp = self.client.get(reverse("clubs-detail", args=(self.club1.code,))) + self.assertIn(resp.status_code, [200, 201], resp.content) + self.assertEqual(len(resp.data["advisor_set"]), 3) + sorted_advisors = sorted( + [advisor["name"] for advisor in resp.data["advisor_set"]] + ) + self.assertEqual( + sorted_advisors, ["Anonymous Avi", "Jocular Julian", "Reclusive Rohan"] + ) + + def test_advisor_viewset(self): + # Make sure we can't view advisors via the viewset if not authed + self.client.logout() + resp = self.client.get(reverse("club-advisors-list", args=(self.club1.code,))) + self.assertIn(resp.status_code, [400, 403], resp.content) + self.assertIn("detail", resp.data) + + self.client.login(username=self.user1.username, password="test") + resp = self.client.get(reverse("club-advisors-list", args=(self.club1.code,))) + self.assertIn(resp.status_code, [400, 403], resp.content) + self.assertIn("detail", resp.data) + + self.client.login(username=self.user5.username, password="test") + resp = self.client.get(reverse("club-advisors-list", args=(self.club1.code,))) + self.assertIn(resp.status_code, [200, 201], resp.content) + self.assertEqual(len(resp.data), 3) + def test_club_upload(self): """ Test uploading a club logo. diff --git a/frontend/components/ClubEditPage/AdvisorCard.tsx b/frontend/components/ClubEditPage/AdvisorCard.tsx index b8762a60f..7be10aae5 100644 --- a/frontend/components/ClubEditPage/AdvisorCard.tsx +++ b/frontend/components/ClubEditPage/AdvisorCard.tsx @@ -1,17 +1,10 @@ import { Field } from 'formik' import { ReactElement, useState } from 'react' -import styled from 'styled-components' -import { RED } from '../../constants/colors' -import { Advisor, Club } from '../../types' -import { - OBJECT_NAME_SINGULAR, - SHOW_MEMBERS, - SITE_ID, - SITE_NAME, -} from '../../utils/branding' +import { Advisor, AdvisorVisibilityType, Club } from '../../types' +import { OBJECT_NAME_SINGULAR, SHOW_MEMBERS } from '../../utils/branding' import { Text } from '../common' -import { CheckboxField, TextField } from '../FormComponents' +import { SelectField, TextField } from '../FormComponents' import { ModelForm } from '../ModelForm' import BaseCard from './BaseCard' @@ -20,10 +13,20 @@ type Props = { validateAdvisors?: (valid: boolean) => void } -const RequireText = styled.p` - color: ${RED}; - margin-top: 1rem; -` +export const VISIBILITY_TYPES = [ + { + value: AdvisorVisibilityType.AdminOnly, + label: 'Admin Only', + }, + { + value: AdvisorVisibilityType.Students, + label: 'Students (Logged In)', + }, + { + value: AdvisorVisibilityType.All, + label: 'All (Public, External)', + }, +] export default function AdvisorCard({ club, @@ -39,7 +42,7 @@ export default function AdvisorCard({ if (newAdvisors.length) { validCount = newAdvisors.filter( (advisor) => - (advisor._status || !advisor._errorMessage) && advisor.public, + (advisor._status || !advisor._errorMessage) && advisor.visibility, ).length } if (validateAdvisors) { @@ -56,19 +59,25 @@ export default function AdvisorCard({ value} + isMulti={false} + valueDeserialize={(val) => + VISIBILITY_TYPES.find((x) => x.value === val) + } /> ) return ( <> - + - Provide points of contact for your organization. These public points - of contact will be shown to the public. + Provide points of contact for your organization. {SHOW_MEMBERS && ( <> {' '} @@ -82,32 +91,8 @@ export default function AdvisorCard({ isPublic, - )} - fields={fields} - /> - {SITE_ID === 'fyh' && advisorsCount <= 0 && ( - - * At least one public point of contact is required. - - )} - - - - - These private points of contact will be shown to only {SITE_NAME}{' '} - administrators. - - !isPublic, - )} + defaultObject={{ public: AdvisorVisibilityType.Students }} + initialData={club.advisor_set} fields={fields} /> diff --git a/frontend/components/ClubPage/QuestionList.tsx b/frontend/components/ClubPage/QuestionList.tsx index bd684856d..e1a7c7518 100644 --- a/frontend/components/ClubPage/QuestionList.tsx +++ b/frontend/components/ClubPage/QuestionList.tsx @@ -39,7 +39,6 @@ const QuestionList = ({ sortBy, }: QuestionListProps): ReactElement => { const [formSubmitted, setFormSubmitted] = useState(false) - const handleSubmit = (data, { setSubmitting, setStatus }) => { doApiRequest(`/clubs/${code}/questions/?format=json`, { method: 'POST', diff --git a/frontend/types.ts b/frontend/types.ts index 7ddb07186..947567c10 100644 --- a/frontend/types.ts +++ b/frontend/types.ts @@ -80,6 +80,12 @@ export interface CountedEventTicket extends EventTicket { count?: number } +export enum AdvisorVisibilityType { + AdminOnly = 1, + Students = 2, + All = 3, +} + export interface ClubApplication { id: number name: string @@ -147,7 +153,7 @@ export interface Advisor { department: string email: string phone: string - public: boolean + visibility: AdvisorVisibilityType } export interface Club { From 7d67257ee9c03e3e2edaf8639e4d54068f0e27b1 Mon Sep 17 00:00:00 2001 From: Thomas Ngulube <47449914+Porcupine1@users.noreply.github.com> Date: Fri, 4 Oct 2024 15:41:00 -0400 Subject: [PATCH 06/45] fix typo --- backend/clubs/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/clubs/serializers.py b/backend/clubs/serializers.py index 865223174..99fe497b7 100644 --- a/backend/clubs/serializers.py +++ b/backend/clubs/serializers.py @@ -2319,7 +2319,7 @@ def get_attribute(self, instance): def to_representation(self, value): """ This is called to get the value for a partcular cell, given the club code. - The entire field object can be though of as a column in the spreadsheet. + The entire field object can be thought of as a column in the spreadsheet. """ return self._cached_values.get(value, self._default_value) From 7218bf666542abcafbebb880a1f231603a7dcc29 Mon Sep 17 00:00:00 2001 From: Julian Weng Date: Fri, 4 Oct 2024 17:05:37 -0400 Subject: [PATCH 07/45] fix another typo --- backend/clubs/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/clubs/serializers.py b/backend/clubs/serializers.py index 99fe497b7..444dfc9ef 100644 --- a/backend/clubs/serializers.py +++ b/backend/clubs/serializers.py @@ -2318,7 +2318,7 @@ def get_attribute(self, instance): def to_representation(self, value): """ - This is called to get the value for a partcular cell, given the club code. + This is called to get the value for a particular cell, given the club code. The entire field object can be thought of as a column in the spreadsheet. """ return self._cached_values.get(value, self._default_value) From 51288a49de3b1345842412050ca040709c7086c2 Mon Sep 17 00:00:00 2001 From: Julian Weng Date: Fri, 4 Oct 2024 22:30:08 -0400 Subject: [PATCH 08/45] Enable paid tickets --- frontend/utils/branding.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/utils/branding.tsx b/frontend/utils/branding.tsx index f423717d9..a37877c60 100644 --- a/frontend/utils/branding.tsx +++ b/frontend/utils/branding.tsx @@ -251,7 +251,7 @@ const sites = { }, } -export const TICKETING_PAYMENT_ENABLED = false +export const TICKETING_PAYMENT_ENABLED = true export const REAPPROVAL_QUEUE_ENABLED = true export const NEW_APPROVAL_QUEUE_ENABLED = true export const SITE_ID = site From d65b3772975899cf1a142637d1db9854885dd68f Mon Sep 17 00:00:00 2001 From: Julian Weng Date: Fri, 4 Oct 2024 22:43:13 -0400 Subject: [PATCH 09/45] Implement beta flag for clubs --- .../0115_club_beta_historicalclub_beta.py | 23 +++++++++++++++++++ backend/clubs/models.py | 1 + backend/clubs/serializers.py | 6 +++++ .../components/ClubEditPage/EventsCard.tsx | 10 +++++--- .../components/ClubEditPage/TicketsModal.tsx | 9 ++++++-- frontend/types.ts | 1 + 6 files changed, 45 insertions(+), 5 deletions(-) create mode 100644 backend/clubs/migrations/0115_club_beta_historicalclub_beta.py diff --git a/backend/clubs/migrations/0115_club_beta_historicalclub_beta.py b/backend/clubs/migrations/0115_club_beta_historicalclub_beta.py new file mode 100644 index 000000000..832c7afdf --- /dev/null +++ b/backend/clubs/migrations/0115_club_beta_historicalclub_beta.py @@ -0,0 +1,23 @@ +# Generated by Django 5.0.4 on 2024-10-05 02:24 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("clubs", "0114_alter_advisor_public"), + ] + + operations = [ + migrations.AddField( + model_name="club", + name="beta", + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name="historicalclub", + name="beta", + field=models.BooleanField(default=False), + ), + ] diff --git a/backend/clubs/models.py b/backend/clubs/models.py index 36cc23044..2c38e28e8 100644 --- a/backend/clubs/models.py +++ b/backend/clubs/models.py @@ -294,6 +294,7 @@ class Club(models.Model): code = models.SlugField(max_length=255, unique=True, db_index=True) active = models.BooleanField(default=False) + beta = models.BooleanField(default=False) # opts club into all beta features name = models.CharField(max_length=255) subtitle = models.CharField(blank=True, max_length=255) terms = models.CharField(blank=True, max_length=1024) diff --git a/backend/clubs/serializers.py b/backend/clubs/serializers.py index 444dfc9ef..015a4c7da 100644 --- a/backend/clubs/serializers.py +++ b/backend/clubs/serializers.py @@ -1502,6 +1502,11 @@ def save(self): """ Override save in order to replace code with slugified name if not specified. """ + if "beta" in self.validated_data: + raise serializers.ValidationError( + "The beta field is not allowed to be set by clubs." + ) + # remove any spaces from the name if "name" in self.validated_data: self.validated_data["name"] = self.validated_data["name"].strip() @@ -1704,6 +1709,7 @@ class Meta(ClubListSerializer.Meta): "approved_by", "approved_comment", "badges", + "beta", "created_at", "description", "events", diff --git a/frontend/components/ClubEditPage/EventsCard.tsx b/frontend/components/ClubEditPage/EventsCard.tsx index 1a741614f..cdd765d91 100644 --- a/frontend/components/ClubEditPage/EventsCard.tsx +++ b/frontend/components/ClubEditPage/EventsCard.tsx @@ -401,7 +401,7 @@ const CreateContainer = styled.div` align-items: center; ` -const CreateTickets = ({ event }: { event: ClubEvent }) => { +const CreateTickets = ({ event, club }: { event: ClubEvent; club: Club }) => { const [show, setShow] = useState(false) const showModal = () => setShow(true) const hideModal = () => setShow(false) @@ -431,7 +431,11 @@ const CreateTickets = ({ event }: { event: ClubEvent }) => { closeModal={hideModal} marginBottom={false} > - + )} @@ -476,7 +480,7 @@ export default function EventsCard({ club }: EventsCardProps): ReactElement { }} /> - +
diff --git a/frontend/components/ClubEditPage/TicketsModal.tsx b/frontend/components/ClubEditPage/TicketsModal.tsx index 7eec51535..12553498e 100644 --- a/frontend/components/ClubEditPage/TicketsModal.tsx +++ b/frontend/components/ClubEditPage/TicketsModal.tsx @@ -14,7 +14,7 @@ import { } from '../../constants/colors' import { BORDER_RADIUS } from '../../constants/measurements' import { BODY_FONT } from '../../constants/styles' -import { ClubEvent } from '../../types' +import { Club, ClubEvent } from '../../types' import { doApiRequest } from '../../utils' import CoverPhoto from '../EventPage/CoverPhoto' @@ -56,6 +56,7 @@ const notify = ( type TicketItemProps = { ticket: Ticket + club: Club onChange?: (ticket: Ticket) => void onDelete?: () => void deletable: boolean @@ -63,6 +64,7 @@ type TicketItemProps = { const TicketItem: React.FC = ({ ticket: propTicket, + club, onChange, onDelete, deletable, @@ -122,7 +124,7 @@ const TicketItem: React.FC = ({ className="input" value={ticket.price ?? ''} placeholder="Ticket Price" - disabled={!TICKETING_PAYMENT_ENABLED} + disabled={!TICKETING_PAYMENT_ENABLED && !club.beta} onChange={(e) => { const price = e.target.value setTicket({ ...ticket, price }) @@ -221,9 +223,11 @@ type Ticket = { const TicketsModal = ({ event, + club, onSuccessfulSubmit, }: { event: ClubEvent + club: Club onSuccessfulSubmit: () => void }): ReactElement => { const { large_image_url, image_url, club_name, name, id } = event @@ -333,6 +337,7 @@ const TicketsModal = ({ 1} onChange={(newTicket) => { setTickets((t) => diff --git a/frontend/types.ts b/frontend/types.ts index 947567c10..00a343f2e 100644 --- a/frontend/types.ts +++ b/frontend/types.ts @@ -167,6 +167,7 @@ export interface Club { approved_comment: string | null available_virtually: boolean badges: Badge[] + beta: boolean code: string description: string email: string From 87beb48bcecfb5a1761860731f02e30a5591a327 Mon Sep 17 00:00:00 2001 From: Rohan Moniz <60864468+rm03@users.noreply.github.com> Date: Fri, 4 Oct 2024 22:44:24 -0400 Subject: [PATCH 10/45] remove frontend check again :skull: --- .github/workflows/build-and-deploy.yaml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/.github/workflows/build-and-deploy.yaml b/.github/workflows/build-and-deploy.yaml index 4e03f9add..cbb6ba54c 100644 --- a/.github/workflows/build-and-deploy.yaml +++ b/.github/workflows/build-and-deploy.yaml @@ -18,13 +18,6 @@ jobs: black: false ruff: true - frontend-check: - name: "Frontend Check" - uses: pennlabs/shared-actions/.github/workflows/react-check.yaml@v0.1 - with: - path: frontend - nodeVersion: 20.11.1 - build-backend: name: Build backend runs-on: ubuntu-latest @@ -79,7 +72,6 @@ jobs: with: name: build-frontend path: /tmp/image.tar - needs: frontend-check publish: name: Publish Images From 521441ef5d3cb892da45dfc5ae3d7ae3fd113149 Mon Sep 17 00:00:00 2001 From: aviupadhyayula Date: Mon, 7 Oct 2024 10:57:25 -0400 Subject: [PATCH 11/45] Add index on club approval date --- .../0116_alter_club_approved_on_and_more.py | 23 +++++++++++++++++++ backend/clubs/models.py | 2 +- 2 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 backend/clubs/migrations/0116_alter_club_approved_on_and_more.py diff --git a/backend/clubs/migrations/0116_alter_club_approved_on_and_more.py b/backend/clubs/migrations/0116_alter_club_approved_on_and_more.py new file mode 100644 index 000000000..d332c9988 --- /dev/null +++ b/backend/clubs/migrations/0116_alter_club_approved_on_and_more.py @@ -0,0 +1,23 @@ +# Generated by Django 5.0.4 on 2024-10-07 14:56 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("clubs", "0115_club_beta_historicalclub_beta"), + ] + + operations = [ + migrations.AlterField( + model_name="club", + name="approved_on", + field=models.DateTimeField(blank=True, db_index=True, null=True), + ), + migrations.AlterField( + model_name="historicalclub", + name="approved_on", + field=models.DateTimeField(blank=True, db_index=True, null=True), + ), + ] diff --git a/backend/clubs/models.py b/backend/clubs/models.py index 2c38e28e8..65a69cb7a 100644 --- a/backend/clubs/models.py +++ b/backend/clubs/models.py @@ -280,7 +280,7 @@ class Club(models.Model): blank=True, ) approved_comment = models.TextField(null=True, blank=True) - approved_on = models.DateTimeField(null=True, blank=True) + approved_on = models.DateTimeField(null=True, blank=True, db_index=True) archived = models.BooleanField(default=False) archived_by = models.ForeignKey( From 59763e4013e081f060481bb89717efea43739529 Mon Sep 17 00:00:00 2001 From: Joy Liu <34288846+joyliu-q@users.noreply.github.com> Date: Tue, 8 Oct 2024 18:27:05 -0400 Subject: [PATCH 12/45] Update main.ts --- k8s/main.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/k8s/main.ts b/k8s/main.ts index 1a592a5f7..6cafd014d 100644 --- a/k8s/main.ts +++ b/k8s/main.ts @@ -31,7 +31,7 @@ export class MyChart extends PennLabsChart { new DjangoApplication(this, 'django-wsgi', { deployment: { image: backendImage, - replicas: 4, + replicas: 2, secret: clubsSecret, env: [ { name: 'REDIS_HOST', value: 'penn-clubs-redis' }, @@ -60,7 +60,7 @@ export class MyChart extends PennLabsChart { new ReactApplication(this, 'react', { deployment: { image: frontendImage, - replicas: 3, + replicas: 2, }, domain: { host: clubsDomain, paths: ['/'] }, port: 80, From 221ee1b18c64f9cc1494cad22d9f34956b8b5efa Mon Sep 17 00:00:00 2001 From: Julian Weng Date: Wed, 9 Oct 2024 09:25:39 -0400 Subject: [PATCH 13/45] Fix style prop errors and attempt version bump + using non-dev api for unified checkout JS library --- frontend/components/Settings/ClubTab.tsx | 2 +- frontend/components/Settings/RenewTab.tsx | 2 +- frontend/components/Settings/TicketsTab/index.tsx | 2 +- frontend/components/Tickets/CartTickets.tsx | 2 +- frontend/components/Tickets/PaymentForm.tsx | 2 +- frontend/components/common/Typography.tsx | 4 ++-- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/frontend/components/Settings/ClubTab.tsx b/frontend/components/Settings/ClubTab.tsx index 114aa0717..08d8a0dd3 100644 --- a/frontend/components/Settings/ClubTab.tsx +++ b/frontend/components/Settings/ClubTab.tsx @@ -232,7 +232,7 @@ const ClubTab = ({ <>
- + No memberships yet! Browse {OBJECT_NAME_PLURAL}{' '} here. diff --git a/frontend/components/Settings/RenewTab.tsx b/frontend/components/Settings/RenewTab.tsx index c1413f2b2..0cbdea7a9 100644 --- a/frontend/components/Settings/RenewTab.tsx +++ b/frontend/components/Settings/RenewTab.tsx @@ -68,7 +68,7 @@ const RenewTab = ({ className }: ClubTabProps): ReactElement => { <>
- + You are not listed as an officer for any {OBJECT_NAME_PLURAL} yet. If you would like to request access for an existing{' '} {OBJECT_NAME_SINGULAR}, please send your name, PennKey, and{' '} diff --git a/frontend/components/Settings/TicketsTab/index.tsx b/frontend/components/Settings/TicketsTab/index.tsx index 650a2d6be..ba74c9884 100644 --- a/frontend/components/Settings/TicketsTab/index.tsx +++ b/frontend/components/Settings/TicketsTab/index.tsx @@ -257,7 +257,7 @@ const TicketsTab = ({ className, userInfo }: TicketsTabProps): ReactElement => { <>
- + No tickets yet! Browse events to find tickets{' '} here. diff --git a/frontend/components/Tickets/CartTickets.tsx b/frontend/components/Tickets/CartTickets.tsx index f593b3097..edab5ce80 100644 --- a/frontend/components/Tickets/CartTickets.tsx +++ b/frontend/components/Tickets/CartTickets.tsx @@ -313,7 +313,7 @@ const CartTickets: React.FC = ({ tickets, soldOut }) => { }} > Your cart is empty - + To add tickets to your cart, visit the event page and select the tickets you wish to purchase. If you believe this is an error, please contact support at diff --git a/frontend/components/Tickets/PaymentForm.tsx b/frontend/components/Tickets/PaymentForm.tsx index d45d5ebad..8333ace82 100644 --- a/frontend/components/Tickets/PaymentForm.tsx +++ b/frontend/components/Tickets/PaymentForm.tsx @@ -36,7 +36,7 @@ const Payment: React.FC = ({ return ( <>