diff --git a/ws/api_views.py b/ws/api_views.py index 825c0894..20a0df7e 100644 --- a/ws/api_views.py +++ b/ws/api_views.py @@ -21,9 +21,10 @@ import ws.utils.signups as signup_utils from ws import enums, models from ws.decorators import group_required +from ws.mixins import JsonTripLeadersOnlyView, TripLeadersOnlyView from ws.templatetags.avatar_tags import avatar_url from ws.utils.api import jwt_token_from_headers -from ws.views import AllLeadersView, TripLeadersOnlyView +from ws.views.leaders import AllLeadersView class SimpleSignupsView(DetailView): @@ -146,7 +147,7 @@ def post(self, request, *args, **kwargs): """ trip = self.object = self.get_object() - postdata = json.loads(self.request.body) + postdata = json.loads(self.request.body) # TODO: assumes valid JSON. signup_list = postdata.get('signups', []) maximum_participants = postdata.get('maximum_participants') @@ -264,7 +265,7 @@ def describe_all_signups(self): class LeaderParticipantSignupView( - SingleObjectMixin, FormatSignupMixin, TripLeadersOnlyView + SingleObjectMixin, FormatSignupMixin, JsonTripLeadersOnlyView ): model = models.Trip @@ -278,6 +279,7 @@ def post(self, request, *args, **kwargs): postdata = json.loads(self.request.body) par_pk = postdata.get('participant_id') + notes = postdata.get('notes', '') try: par = models.Participant.objects.get(pk=par_pk) @@ -286,7 +288,7 @@ def post(self, request, *args, **kwargs): trip = self.get_object() signup, created = models.SignUp.objects.get_or_create( - trip=trip, participant=par + trip=trip, participant=par, defaults={'notes': notes} ) if not created: # (SignUp exists, but participant may not be on trip) @@ -301,7 +303,6 @@ def post(self, request, *args, **kwargs): {'message': f"{par.name} is already on the {queue}"}, status=409 ) - signup.notes = postdata.get('notes', '') signup = signup_utils.trip_or_wait(signup) trip_participants = { @@ -322,7 +323,7 @@ def post(self, request, *args, **kwargs): 'signup': self.describe_signup(signup, trip_participants, other_trips), 'on_trip': signup.on_trip, }, - status=201, + status=201 if (created or signup.on_trip) else 200, ) diff --git a/ws/mixins.py b/ws/mixins.py index de909fc9..7e5ef777 100644 --- a/ws/mixins.py +++ b/ws/mixins.py @@ -3,7 +3,7 @@ """ from django.contrib.auth.decorators import login_required from django.db.models import Q -from django.http import HttpRequest +from django.http import HttpRequest, JsonResponse from django.shortcuts import render from django.utils.decorators import method_decorator from django.views.generic import View @@ -69,19 +69,33 @@ def can_set_attendance(self, participant_to_modify) -> bool: return participant_to_modify.user_id == self.request.user.id +def _allowed_to_modify_trip(trip: models.Trip, request: HttpRequest) -> bool: + activity_enum = trip.required_activity_enum() + if activity_enum: + is_chair = perm_utils.chair_or_admin(request.user, activity_enum) + else: # (There is no required activity, so no chairs. Allow superusers, though) + is_chair = request.user.is_superuser + + participant: models.Participant = request.participant # type: ignore + trip_leader = perm_utils.leader_on_trip(participant, trip, True) + return is_chair or trip_leader + + class TripLeadersOnlyView(View): @method_decorator(login_required) def dispatch(self, request, *args, **kwargs): """Only allow creator, leaders of the trip, and chairs.""" + trip = self.get_object() + if not _allowed_to_modify_trip(trip, request): + return render(request, 'not_your_trip.html', {'trip': trip}) + return super().dispatch(request, *args, **kwargs) - activity_enum = trip.required_activity_enum() - if activity_enum: - is_chair = perm_utils.chair_or_admin(request.user, activity_enum) - else: # (There is no required activity, so no chairs. Allow superusers, though) - is_chair = request.user.is_superuser - trip_leader = perm_utils.leader_on_trip(request.participant, trip, True) - if not (is_chair or trip_leader): - return render(request, 'not_your_trip.html', {'trip': trip}) +class JsonTripLeadersOnlyView(View): + @method_decorator(login_required) + def dispatch(self, request, *args, **kwargs): + """Only allow creator, leaders of the trip, and chairs.""" + if not _allowed_to_modify_trip(self.get_object(), request): + return JsonResponse({"message": "Must be a leader"}, status=403) return super().dispatch(request, *args, **kwargs) diff --git a/ws/tests/views/test_api_views.py b/ws/tests/views/test_api_views.py index 6f5f6368..b66582a6 100644 --- a/ws/tests/views/test_api_views.py +++ b/ws/tests/views/test_api_views.py @@ -5,8 +5,9 @@ from freezegun import freeze_time import ws.utils.perms as perm_utils -from ws import enums, settings +from ws import enums, models, settings from ws.tests import TestCase, factories +from ws.utils.signups import add_to_waitlist class JWTSecurityTest(TestCase): @@ -383,6 +384,175 @@ def test_exact_id(self): ) +class JsonLeaderParticipantSignupTest(TestCase): + def setUp(self): + super().setUp() + self.trip = factories.TripFactory.create(algorithm='lottery') + self.url = f'/trips/{self.trip.pk}/signup/' + self.leader = factories.ParticipantFactory.create() + self.trip.leaders.add(self.leader) + self.client.force_login(self.leader.user) + + def test_just_user(self): + """A user without a participant record gets a 403 trying to sign up a user.""" + user = factories.UserFactory.create() + self.client.force_login(user) + response = self.client.post(self.url) + self.assertEqual(response.status_code, 403) + + def test_unauthenticated(self): + """One must obviously be logged in to sign up others.""" + self.client.logout() + response = self.client.post(self.url) + # This is a JSON route - it probably shouldn't return with a redirect. Ah, well. + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, f"/accounts/login/?next={self.url}") + + def test_not_a_leader(self): + """Participants must be a leader for the trip in question.""" + par = factories.ParticipantFactory.create() + self.client.force_login(par.user) + response = self.client.post(self.url) + self.assertEqual(response.status_code, 403) + + def test_leader_but_not_on_the_trip(self): + """Participants must be a leader for the trip in question.""" + other_trip = factories.TripFactory.create() + response = self.client.post(f'/trips/{other_trip.pk}/signup/') + self.assertEqual(response.status_code, 403) + + def test_unknown_target_participant(self): + """If the caller is a leader, we'll tell them if the participant isn't found.""" + response = self.client.post( + self.url, + {'participant_id': -37}, + content_type='application/json', + ) + self.assertEqual(response.status_code, 404) + self.assertEqual(response.json(), {'message': "No participant found"}) + + def test_signup_exists_but_not_on_lottery_trip(self): + """We won't edit notes and the trip remains in lottery mode.""" + signup = factories.SignUpFactory.create( + on_trip=False, trip=self.trip, notes='original, participant-supplied notes' + ) + response = self.client.post( + self.url, + {'participant_id': signup.participant.pk, 'notes': 'leader notes'}, + content_type='application/json', + ) + + # Participant was already signed up for the lottery + self.assertEqual(self.trip.algorithm, 'lottery') + self.assertEqual(response.status_code, 200) + signup.refresh_from_db() + self.assertFalse(signup.on_trip) + self.assertEqual(signup.notes, 'original, participant-supplied notes') + + def test_signup_exists_but_not_on_fcfs_trip(self): + """We won't edit notes, but we can add the participant to the trip.""" + signup = factories.SignUpFactory.create( + on_trip=False, trip=self.trip, notes='original, participant-supplied notes' + ) + + self.trip.algorithm = 'fcfs' + self.trip.save() + + response = self.client.post( + self.url, + {'participant_id': signup.participant.pk, 'notes': 'leader notes'}, + content_type='application/json', + ) + + self.assertEqual(response.status_code, 201) + signup.refresh_from_db() + self.assertTrue(signup.on_trip) + self.assertEqual(signup.notes, 'original, participant-supplied notes') + + def test_new_signup_to_fcfs_trip(self): + """Test the simplest case: adding a participant to a FCFS trip with spaces.""" + self.trip.algorithm = 'fcfs' + self.trip.save() + + par = factories.ParticipantFactory.create() + + response = self.client.post( + self.url, + {'participant_id': par.pk, 'notes': 'leader notes'}, + content_type='application/json', + ) + + self.assertEqual(response.status_code, 201) + signup = models.SignUp.objects.get(participant=par, trip=self.trip) + self.assertTrue(signup.on_trip) + self.assertEqual(signup.notes, 'leader notes') + + def test_add_new_waitlist_entry(self): + """We can add a participant to the waitlist for a full trip.""" + self.trip.algorithm = 'fcfs' + self.trip.maximum_participants = 1 + self.trip.save() + + # Trip is full now + factories.SignUpFactory.create(trip=self.trip) + + par = factories.ParticipantFactory.create() + + response = self.client.post( + self.url, + {'participant_id': par.pk}, + content_type='application/json', + ) + + self.assertEqual(response.status_code, 201) + signup = models.SignUp.objects.get(participant=par, trip=self.trip) + self.assertFalse(signup.on_trip) + self.assertTrue(signup.waitlistsignup) + self.assertEqual(signup.notes, '') + + def test_already_on_trip(self): + """Leaders can't add a participant already on the trip!""" + signup = factories.SignUpFactory.create( + trip=self.trip, on_trip=True, participant__name='Abdul McTest' + ) + + response = self.client.post( + self.url, + {'participant_id': signup.participant_id}, + content_type='application/json', + ) + + self.assertEqual(response.status_code, 409) + self.assertEqual( + response.json(), {'message': "Abdul McTest is already on the trip"} + ) + + def test_already_on_waitlist(self): + """Leaders can't add a participant already on the waitlist!""" + self.trip.algorithm = 'fcfs' + self.trip.maximum_participants = 1 + self.trip.save() + + # Trip is full now + factories.SignUpFactory.create(trip=self.trip) + + signup = factories.SignUpFactory.create( + trip=self.trip, participant__name='Jane McJaney' + ) + add_to_waitlist(signup) + + response = self.client.post( + self.url, + {'participant_id': signup.participant_id}, + content_type='application/json', + ) + + self.assertEqual(response.status_code, 409) + self.assertEqual( + response.json(), {'message': "Jane McJaney is already on the waitlist"} + ) + + class ApproveTripViewTest(TestCase): def setUp(self): super().setUp()