From 49de27cf7abec78e7494508d2aa1ad77c5705d38 Mon Sep 17 00:00:00 2001 From: JasonGrace2282 Date: Thu, 19 Sep 2024 13:50:49 -0400 Subject: [PATCH] feat: implement individual students stickying Closes #1719 --- docs/sourcedoc/intranet.apps.eighth.forms.rst | 8 ++ intranet/apps/dashboard/views.py | 2 +- .../apps/eighth/forms/admin/scheduling.py | 8 ++ intranet/apps/eighth/forms/fields.py | 26 ++++++ ...eighthscheduledactivity_sticky_students.py | 20 +++++ intranet/apps/eighth/models.py | 81 ++++++++++++++++--- intranet/apps/eighth/serializers.py | 8 +- intranet/apps/eighth/tests/test_signup.py | 52 ++++++++++++ intranet/apps/eighth/urls.py | 1 + .../apps/eighth/views/admin/scheduling.py | 11 ++- intranet/apps/eighth/views/admin/users.py | 26 +++++- .../eighth/admin/schedule_activity.html | 19 ++++- .../eighth/emails/students_stickied.html | 13 +++ .../eighth/emails/students_stickied.txt | 1 + .../eighth/emails/students_unstickied.html | 16 ++++ .../eighth/emails/students_unstickied.txt | 4 + intranet/templates/eighth/signup.html | 6 +- 17 files changed, 282 insertions(+), 20 deletions(-) create mode 100644 intranet/apps/eighth/forms/fields.py create mode 100644 intranet/apps/eighth/migrations/0071_eighthscheduledactivity_sticky_students.py create mode 100644 intranet/templates/eighth/emails/students_stickied.html create mode 100644 intranet/templates/eighth/emails/students_stickied.txt create mode 100644 intranet/templates/eighth/emails/students_unstickied.html create mode 100644 intranet/templates/eighth/emails/students_unstickied.txt diff --git a/docs/sourcedoc/intranet.apps.eighth.forms.rst b/docs/sourcedoc/intranet.apps.eighth.forms.rst index a512b0cc269..05d9ad08c3d 100644 --- a/docs/sourcedoc/intranet.apps.eighth.forms.rst +++ b/docs/sourcedoc/intranet.apps.eighth.forms.rst @@ -20,6 +20,14 @@ intranet.apps.eighth.forms.activities module :undoc-members: :show-inheritance: +intranet.apps.eighth.forms.fields module +---------------------------------------- + +.. automodule:: intranet.apps.eighth.forms.fields + :members: + :undoc-members: + :show-inheritance: + Module contents --------------- diff --git a/intranet/apps/dashboard/views.py b/intranet/apps/dashboard/views.py index 0e6a15514d2..6ade7f1a5a0 100644 --- a/intranet/apps/dashboard/views.py +++ b/intranet/apps/dashboard/views.py @@ -72,7 +72,7 @@ def gen_schedule(user, num_blocks: int = 6, surrounding_blocks: Iterable[EighthB if current_sched_act: current_signup = current_sched_act.title_with_flags current_signup_cancelled = current_sched_act.cancelled - current_signup_sticky = current_sched_act.activity.sticky + current_signup_sticky = current_sched_act.is_user_stickied(user) rooms = current_sched_act.get_true_rooms() else: current_signup = None diff --git a/intranet/apps/eighth/forms/admin/scheduling.py b/intranet/apps/eighth/forms/admin/scheduling.py index d0a8d400ac3..6da24c5941c 100644 --- a/intranet/apps/eighth/forms/admin/scheduling.py +++ b/intranet/apps/eighth/forms/admin/scheduling.py @@ -1,6 +1,8 @@ from django import forms +from django.contrib.auth import get_user_model from ...models import EighthScheduledActivity +from .. import fields class ScheduledActivityForm(forms.ModelForm): @@ -20,6 +22,12 @@ def __init__(self, *args, **kwargs): for fieldname in ["block", "activity"]: self.fields[fieldname].widget = forms.HiddenInput() + self.fields["sticky_students"] = fields.UserMultipleChoiceField( + queryset=self.initial.get("sticky_students", get_user_model().objects.none()), + required=False, + widget=forms.SelectMultiple(attrs={"class": "remote-source remote-sticky-students"}), + ) + def validate_unique(self): # We'll handle this ourselves by updating if already exists pass diff --git a/intranet/apps/eighth/forms/fields.py b/intranet/apps/eighth/forms/fields.py new file mode 100644 index 00000000000..2bec0d78ff4 --- /dev/null +++ b/intranet/apps/eighth/forms/fields.py @@ -0,0 +1,26 @@ +from django import forms +from django.contrib.auth import get_user_model +from django.core.validators import ValidationError + + +class UserMultipleChoiceField(forms.ModelMultipleChoiceField): + """Choose any user from the database.""" + + def clean(self, value): + if not value and not self.required: + return self.queryset.none() + elif self.required: + raise ValidationError(self.error_messages["required"], code="required") + + try: + users = get_user_model().objects.filter(id__in=value) + if len(users) != len(value): + raise ValidationError(self.error_messages["invalid_choice"], code="invalid_choice") + except (ValueError, TypeError) as e: + raise ValidationError(self.error_messages["invalid_choice"], code="invalid_choice") from e + return users + + def label_from_instance(self, obj): + if isinstance(obj, get_user_model()): + return f"{obj.get_full_name()} ({obj.username})" + return super().label_from_instance(obj) diff --git a/intranet/apps/eighth/migrations/0071_eighthscheduledactivity_sticky_students.py b/intranet/apps/eighth/migrations/0071_eighthscheduledactivity_sticky_students.py new file mode 100644 index 00000000000..3895a17f068 --- /dev/null +++ b/intranet/apps/eighth/migrations/0071_eighthscheduledactivity_sticky_students.py @@ -0,0 +1,20 @@ +# Generated by Django 3.2.25 on 2024-10-12 21:12 + +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('eighth', '0070_eighthactivity_club_sponsors'), + ] + + operations = [ + migrations.AddField( + model_name='eighthscheduledactivity', + name='sticky_students', + field=models.ManyToManyField(blank=True, related_name='sticky_scheduledactivity_set', to=settings.AUTH_USER_MODEL), + ), + ] diff --git a/intranet/apps/eighth/models.py b/intranet/apps/eighth/models.py index 264aba5386e..009c9757b0e 100644 --- a/intranet/apps/eighth/models.py +++ b/intranet/apps/eighth/models.py @@ -2,11 +2,13 @@ import datetime import logging import string +from collections.abc import Sequence from typing import Collection, Iterable, List, Optional, Union from cacheops import invalidate_obj from django.conf import settings from django.contrib.auth import get_user_model +from django.contrib.auth.models import AbstractBaseUser from django.contrib.auth.models import Group as DjangoGroup from django.core import validators from django.core.cache import cache @@ -807,6 +809,11 @@ class EighthScheduledActivity(AbstractBaseEighthModel): activity = models.ForeignKey(EighthActivity, on_delete=models.CASCADE) members = models.ManyToManyField(settings.AUTH_USER_MODEL, through="EighthSignup", related_name="eighthscheduledactivity_set") waitlist = models.ManyToManyField(settings.AUTH_USER_MODEL, through="EighthWaitlist", related_name="%(class)s_scheduledactivity_set") + sticky_students = models.ManyToManyField( + settings.AUTH_USER_MODEL, + related_name="sticky_scheduledactivity_set", + blank=True, + ) admin_comments = models.CharField(max_length=1000, blank=True) title = models.CharField(max_length=1000, blank=True) @@ -862,6 +869,24 @@ def title_with_flags(self) -> str: name_with_flags = "Special: " + name_with_flags return name_with_flags + def is_user_stickied(self, user: AbstractBaseUser) -> bool: + """Check if the given user is stickied to this activity. + + Args: + user: The user to check for stickiness. + """ + return self.is_activity_sticky() or self.sticky_students.filter(pk=user.pk).exists() + + def is_activity_sticky(self) -> bool: + """Check if the scheduled activity or activity is sticky + + .. warning:: + + This method does NOT take into account individual user stickies. + In 99.9% of cases, you should use :meth:`is_user_stickied` instead. + """ + return self.sticky or self.activity.sticky + def get_true_sponsors(self) -> Union[QuerySet, Collection[EighthSponsor]]: # pylint: disable=unsubscriptable-object """Retrieves the sponsors for the scheduled activity, taking into account activity defaults and overrides. @@ -920,13 +945,6 @@ def get_restricted(self) -> bool: """ return self.restricted or self.activity.restricted - def get_sticky(self) -> bool: - """Gets whether this scheduled activity is sticky. - Returns: - Whether this scheduled activity is sticky. - """ - return self.sticky or self.activity.sticky - def get_finance(self) -> str: """Retrieves the name of this activity's account with the finance office, if any. @@ -1097,10 +1115,50 @@ def notify_waitlist(self, waitlists: Iterable["EighthWaitlist"]): [waitlist.user.primary_email_address], ) + def set_sticky_students(self, users: "Sequence[AbstractBaseUser]") -> None: + """Sets the given users to the sticky students list for this activity. + + This also sends emails to students. + + Args: + users: The users to add to the sticky students list. + + Returns: + A tuple of the new stickied students and the unstickied students. + """ + for user in users: + signup = EighthSignup.objects.filter(user=user, scheduled_activity__block=self.block).first() + if signup is not None: + signup.remove_signup(user, force=True) + self.add_user(user, force=True) + + old_sticky_students = self.sticky_students.all() + self.sticky_students.set(users) + + # note: this will send separate emails to each student for each activity they are stickied in + new_stickied_students = [user.notification_email for user in users if user not in old_sticky_students] + unstickied_students = [user.notification_email for user in old_sticky_students if user not in users] + email_send_task.delay( + "eighth/emails/students_stickied.txt", + "eighth/emails/students_stickied.html", + data={"activity": self}, + subject="You have been stickied into an activity", + emails=new_stickied_students, + bcc=True, + ) + email_send_task.delay( + "eighth/emails/students_unstickied.txt", + "eighth/emails/students_unstickied.html", + data={"activity": self}, + subject="You have been unstickied from an activity", + emails=unstickied_students, + bcc=True, + ) + @transaction.atomic # This MUST be run in a transaction. Do NOT remove this decorator. def add_user( self, - user: "get_user_model()", + user: AbstractBaseUser, request: Optional[HttpRequest] = None, force: bool = False, no_after_deadline: bool = False, @@ -1160,8 +1218,9 @@ def add_user( if ( EighthSignup.objects.filter(user=user, scheduled_activity__block__in=all_blocks) .filter(Q(scheduled_activity__activity__sticky=True) | Q(scheduled_activity__sticky=True)) - .filter(Q(scheduled_activity__cancelled=False)) + .filter(scheduled_activity__cancelled=False) .exists() + or user.sticky_scheduledactivity_set.filter(block__in=all_blocks, cancelled=False).exists() ): exception.Sticky = True @@ -1223,7 +1282,7 @@ def add_user( if self.activity.users_blacklisted.filter(username=user).exists(): exception.Blacklisted = True - if self.get_sticky(): + if self.is_user_stickied(user): EighthWaitlist.objects.filter(user_id=user.id, block_id=self.block.id).delete() success_message = "Successfully added to waitlist for activity." if waitlist else "Successfully signed up for activity." @@ -1697,7 +1756,7 @@ def remove_signup(self, user: "get_user_model()" = None, force: bool = False, do exception.ActivityDeleted = True # Check if the user is already stickied into an activity - if self.scheduled_activity.activity and self.scheduled_activity.activity.sticky and not self.scheduled_activity.cancelled: + if self.scheduled_activity.activity and self.scheduled_activity.is_user_stickied(user) and not self.scheduled_activity.cancelled: exception.Sticky = True if exception.messages() and not force: diff --git a/intranet/apps/eighth/serializers.py b/intranet/apps/eighth/serializers.py index 7ce1fb8828b..21418c0f1b2 100644 --- a/intranet/apps/eighth/serializers.py +++ b/intranet/apps/eighth/serializers.py @@ -112,7 +112,10 @@ def process_scheduled_activity( if scheduled_activity.title: prefix += " - " + scheduled_activity.title middle = " (R)" if restricted_for_user else "" - suffix = " (S)" if activity.sticky else "" + if user is not None and scheduled_activity.is_user_stickied(user): + suffix = " (S)" + else: + suffix = "" suffix += " (BB)" if scheduled_activity.is_both_blocks() else "" suffix += " (A)" if activity.administrative else "" suffix += " (Deleted)" if activity.deleted else "" @@ -151,7 +154,8 @@ def process_scheduled_activity( "administrative": scheduled_activity.get_administrative(), "presign": activity.presign, "presign_time": scheduled_activity.is_too_early_to_signup()[1].strftime("%A, %B %-d at %-I:%M %p"), - "sticky": scheduled_activity.get_sticky(), + "sticky": scheduled_activity.is_activity_sticky(), + "user_sticky": scheduled_activity.is_user_stickied(user), "finance": "", # TODO: refactor JS to remove this "title": scheduled_activity.title, "comments": scheduled_activity.comments, # TODO: refactor JS to remove this diff --git a/intranet/apps/eighth/tests/test_signup.py b/intranet/apps/eighth/tests/test_signup.py index 23502f7ecca..6018fcec30b 100644 --- a/intranet/apps/eighth/tests/test_signup.py +++ b/intranet/apps/eighth/tests/test_signup.py @@ -200,6 +200,58 @@ def test_signup_restricitons(self): self.assertEqual(len(EighthScheduledActivity.objects.get(block=block1.id, activity=act1.id).members.all()), 1) self.assertEqual(len(EighthScheduledActivity.objects.get(block=block1.id, activity=act2.id).members.all()), 0) + def test_user_stickied(self): + """Test that stickying an individual user into an activity works.""" + self.make_admin() + user = get_user_model().objects.create(username="user1", graduation_year=get_senior_graduation_year()) + + block = self.add_block(date="2024-09-09", block_letter="A") + room = self.add_room(name="room1", capacity=1) + act = self.add_activity(name="Test Activity 1", restricted=True, users_allowed=[user]) + act.rooms.add(room) + + schact = EighthScheduledActivity.objects.create(block=block, activity=act, capacity=5) + schact.set_sticky_students([user]) + + act2 = self.add_activity(name="Test Activity 2") + act2.rooms.add(room) + schact2 = EighthScheduledActivity.objects.create(block=block, activity=act2, capacity=5) + + # ensure that the user can't sign up to something else + with self.assertRaisesMessage(SignupException, "Sticky"): + self.verify_signup(user, schact2) + + self.client.post(reverse("eighth_signup"), data={"uid": user.id, "bid": block.id, "aid": act2.id}) + self.assertFalse(schact2.members.exists()) + + def test_set_sticky_students(self): + """Test :meth:`~.EighthScheduledActivity.set_sticky_students`.""" + self.make_admin() + user = get_user_model().objects.create(username="user1", graduation_year=get_senior_graduation_year()) + + block = self.add_block(date="2024-09-09", block_letter="A") + room = self.add_room(name="room1", capacity=1) + + old_act = self.add_activity(name="Test Activity 2") + old_act.rooms.add(room) + old_schact = EighthScheduledActivity.objects.create(block=block, activity=old_act, capacity=5) + + old_schact.add_user(user) + + act = self.add_activity(name="Test Activity 1", restricted=True, users_allowed=[user]) + act.rooms.add(room) + + schact = EighthScheduledActivity.objects.create(block=block, activity=act, capacity=5) + schact.set_sticky_students([user]) + + self.assertEqual(1, EighthSignup.objects.filter(user=user, scheduled_activity=schact).count()) + self.assertEqual(0, old_schact.members.count()) + + # and they shouldn't be able to change back to their old activity + self.client.post(reverse("eighth_signup"), data={"uid": user.id, "bid": block.id, "aid": old_act.id}) + self.assertEqual(0, EighthSignup.objects.filter(user=user, scheduled_activity=old_schact).count()) + self.assertEqual(0, old_schact.members.count()) + def test_eighth_signup_view(self): """Tests :func:`~intranet.apps.eighth.views.signup.eighth_signup_view`.""" diff --git a/intranet/apps/eighth/urls.py b/intranet/apps/eighth/urls.py index 2d47e43df7f..1663c781681 100644 --- a/intranet/apps/eighth/urls.py +++ b/intranet/apps/eighth/urls.py @@ -67,6 +67,7 @@ re_path(r"^blocks/delete/(?P\d+)$", blocks.delete_block_view, name="eighth_admin_delete_block"), # Users re_path(r"^users$", users.list_user_view, name="eighth_admin_manage_users"), + re_path(r"^users/non-graduated$", users.list_non_graduated_view, name="eighth_admin_manage_non_graduated"), re_path(r"^users/delete/(\d+)$", users.delete_user_view, name="eighth_admin_manage_users"), # Scheduling re_path(r"^scheduling/schedule$", scheduling.schedule_activity_view, name="eighth_admin_schedule_activity"), diff --git a/intranet/apps/eighth/views/admin/scheduling.py b/intranet/apps/eighth/views/admin/scheduling.py index abc5b4e7372..0c2c895a062 100644 --- a/intranet/apps/eighth/views/admin/scheduling.py +++ b/intranet/apps/eighth/views/admin/scheduling.py @@ -147,7 +147,10 @@ def schedule_activity_view(request): messages.error(request, f"Did not unschedule {name} because there is {count} student signed up.") else: messages.error(request, f"Did not unschedule {name} because there are {count} students signed up.") - instance.save() + + if instance: + instance.save() + instance.set_sticky_students(form.cleaned_data["sticky_students"]) messages.success(request, "Successfully updated schedule.") @@ -201,7 +204,10 @@ def schedule_activity_view(request): initial_formset_data = [] sched_act_queryset = ( - EighthScheduledActivity.objects.filter(activity=activity).select_related("block").prefetch_related("rooms", "sponsors", "members") + EighthScheduledActivity.objects.filter(activity=activity) + .select_related("block") + .prefetch_related("rooms", "sponsors", "members", "sticky_students") + .all() ) all_sched_acts = {sa.block.id: sa for sa in sched_act_queryset} @@ -227,6 +233,7 @@ def schedule_activity_view(request): "admin_comments": sched_act.admin_comments, "scheduled": not sched_act.cancelled, "cancelled": sched_act.cancelled, + "sticky_students": sched_act.sticky_students.all(), } ) except KeyError: diff --git a/intranet/apps/eighth/views/admin/users.py b/intranet/apps/eighth/views/admin/users.py index bfd01c64e81..ca88e668a35 100644 --- a/intranet/apps/eighth/views/admin/users.py +++ b/intranet/apps/eighth/views/admin/users.py @@ -1,9 +1,11 @@ import logging from django.contrib.auth import get_user_model -from django.http import Http404 +from django.http import Http404, JsonResponse from django.shortcuts import get_object_or_404, render +from intranet.utils.date import get_senior_graduation_year + from ....auth.decorators import eighth_admin_required logger = logging.getLogger(__name__) @@ -15,6 +17,28 @@ def list_user_view(request): return render(request, "eighth/admin/list_users.html", {"users": users}) +@eighth_admin_required +def list_non_graduated_view(request): + query = get_user_model().objects.filter( + graduation_year__gte=get_senior_graduation_year(), + ) + user_type = request.GET.get("user_type") + if user_type in {name for name, _ in get_user_model().USER_TYPES}: + query = query.filter(user_type=user_type) + + return JsonResponse( + { + "users": [ + { + "id": user.id, + "name": f"{user.get_full_name()} ({user.username})", + } + for user in query + ], + } + ) + + @eighth_admin_required def delete_user_view(request, pk): user = get_object_or_404(get_user_model(), pk=pk) diff --git a/intranet/templates/eighth/admin/schedule_activity.html b/intranet/templates/eighth/admin/schedule_activity.html index ecb1ecad101..ee1499ec559 100644 --- a/intranet/templates/eighth/admin/schedule_activity.html +++ b/intranet/templates/eighth/admin/schedule_activity.html @@ -59,6 +59,22 @@ $("#admin_comments_badge").click(function(){ $("#admin_comments_modal").slideToggle(); }); + + $.get({ + url: "{% url 'eighth_admin_manage_non_graduated' %}", + data: {"user_type": "student"}, + success: function(response) { + $("select.remote-sticky-students").each((_, el) => { + $(el).selectize({ + plugins: ["remove_button"], + valueField: 'id', + labelField: 'name', + searchField: 'name', + options: response.users, + }) + }); + } + }); }); @@ -216,6 +232,7 @@

Select an Activity:

Comments Admin Comments + Sticky Students @@ -295,7 +312,7 @@

Select an Activity:

{% endif %} - {% if field.name in "rooms capacity sponsors title special administrative restricted sticky both_blocks comments admin_comments" %} + {% if field.name in "rooms capacity sponsors title special administrative restricted sticky both_blocks comments admin_comments sticky_students " %} diff --git a/intranet/templates/eighth/emails/students_stickied.html b/intranet/templates/eighth/emails/students_stickied.html new file mode 100644 index 00000000000..d0972ec97ef --- /dev/null +++ b/intranet/templates/eighth/emails/students_stickied.html @@ -0,0 +1,13 @@ + + + {% with description="Eighth Signup Changes" %} + {% include "email_metadata.html" %} + {% endwith %} + +

+ You have been stickied into {{ activity.activity.name }} on {{ activity.date }} ({{ activity.block_letter }}). +

+ + {% include "email_footer.txt" %} + + diff --git a/intranet/templates/eighth/emails/students_stickied.txt b/intranet/templates/eighth/emails/students_stickied.txt new file mode 100644 index 00000000000..7bf05cb288d --- /dev/null +++ b/intranet/templates/eighth/emails/students_stickied.txt @@ -0,0 +1 @@ +You have been stickied into {{ activity.activity.name }} on {{ activity.date }} ({{ activity.block_letter }}). diff --git a/intranet/templates/eighth/emails/students_unstickied.html b/intranet/templates/eighth/emails/students_unstickied.html new file mode 100644 index 00000000000..6c2d50b3fbf --- /dev/null +++ b/intranet/templates/eighth/emails/students_unstickied.html @@ -0,0 +1,16 @@ + + + {% with description="Eighth Signup Changes" %} + {% include "email_metadata.html" %} + {% endwith %} + +

+ You have been unstickied from {{ activity.activity.name }} on {{ activity.date }} ({{ activity.block_letter }}). + Please select a new eighth period to sign up for. +
+ Have a nice day! +

+ + {% include "email_footer.txt" %} + + diff --git a/intranet/templates/eighth/emails/students_unstickied.txt b/intranet/templates/eighth/emails/students_unstickied.txt new file mode 100644 index 00000000000..fb14b978dff --- /dev/null +++ b/intranet/templates/eighth/emails/students_unstickied.txt @@ -0,0 +1,4 @@ +You have been unstickied from {{ activity.activity.name }} on {{ activity.date }} ({{ activity.block_letter }}). +Please select a new eighth period to sign up for. + +Have a nice day! diff --git a/intranet/templates/eighth/signup.html b/intranet/templates/eighth/signup.html index c8dc7af3376..d07dbdd6eed 100644 --- a/intranet/templates/eighth/signup.html +++ b/intranet/templates/eighth/signup.html @@ -211,7 +211,9 @@

Authorized <% } %> - <% if (sticky && (selected || isEighthAdmin)) { %> + <% if (user_sticky && (selected || isEighthAdmin)) { %> + Individual Sticky + <% } else if (sticky && (selected || isEighthAdmin)) { %> Sticky <% } %> @@ -546,4 +548,4 @@