diff --git a/routes/event_router.py b/routes/event_router.py index 61cd92ae..f134d5f9 100644 --- a/routes/event_router.py +++ b/routes/event_router.py @@ -220,15 +220,26 @@ def create_event_signup_list(event_id: int, db: DB_dependency): if person.priority in priorites: prioritized_people.append(person) - places_left = event.max_event_users - len(prioritized_people) + # If we are going to hit a limit with the prioritized people, hand out slots based on sorting methods + if len(prioritized_people) > event.max_event_users: + if event.lottery: + random.seed(event_id) + random.shuffle(prioritized_people) + else: + prioritized_people.sort(key=lambda p: p.created_at) - if event.lottery: + prioritized_people = prioritized_people[: event.max_event_users] + + # Floor at 0 to not lie about how many places are left + places_left = max(event.max_event_users - len(prioritized_people), 0) + + if event.lottery and places_left > 0: # Random fill non_prioritized = [p for p in people_signups if p not in prioritized_people] random.seed(event_id) random.shuffle(non_prioritized) prioritized_people.extend(non_prioritized[:places_left]) - else: + elif places_left > 0: # FIFO fill non_prioritized = ( db.query(EventUser_DB).filter_by(event_id=event_id).order_by(EventUser_DB.created_at.asc()).all() diff --git a/services/event_signup_service.py b/services/event_signup_service.py index 0bab8fef..2185641a 100644 --- a/services/event_signup_service.py +++ b/services/event_signup_service.py @@ -15,14 +15,18 @@ def get_allowed_signup_priorities(event: Event_DB, user: User_DB, db: Session) -> set[str]: """The priorities the user may sign up to the event with: the event's own priorities which the - user actually holds, plus the default priority which everyone falls back on.""" + user actually holds, or else the default priority which everyone falls back on. + This effectively blocks signups with the default priority, if a user has a stronger one.""" allowed = {DEFAULT_USER_PRIORITY} if not event.priorities: return allowed user_priorities = {post.name_sv for post in user.posts} | get_user_nollning_priorities(db, user) + matching = {p.priority for p in event.priorities} & user_priorities - return allowed | ({p.priority for p in event.priorities} & user_priorities) + # Someone who holds one of the event's priorities has to sign up with it, since the default + # priority would silently cost them their place when the spots are handed out. + return matching or allowed def check_priority_allowed(event: Event_DB, user: User_DB, priority: str, db: Session): @@ -68,8 +72,8 @@ def signup_to_event(event: Event_DB, user: User_DB, data: EventSignupCreate, man if manage_permission == False and not is_group_allowed(event, user, data.group_name): raise HTTPException(status.HTTP_403_FORBIDDEN, detail="User cannot sign up with this group") - if manage_permission == False and data.priority: # a falsy priority just means the default one - check_priority_allowed(event, user, data.priority, db) + if manage_permission == False: # a falsy priority is stored as, and checked as, the default one + check_priority_allowed(event, user, data.priority or DEFAULT_USER_PRIORITY, db) signup = EventUser_DB(user=user, user_id=user.id, event=event, event_id=event.id) diff --git a/tests/test_event_signup.py b/tests/test_event_signup.py index 085cb01a..5dad633d 100644 --- a/tests/test_event_signup.py +++ b/tests/test_event_signup.py @@ -8,7 +8,7 @@ def test_signup_with_allowed_group_type(client, member_token, membered_user, nol """A group whose type is in mentor_group_types is accepted.""" response = client.post( f"/event-signup/{nollning_event['id']}", - json={"user_id": membered_user.id, "group_name": mentor_group.name}, + json={"user_id": membered_user.id, "group_name": mentor_group.name, "priority": "Nolla"}, headers=auth_headers(member_token), ) @@ -76,7 +76,7 @@ def test_nollning_event_signup_without_group_with_post_priority( response = client.post( f"/event-signup/{event['id']}", - json={"user_id": membered_user.id}, + json={"user_id": membered_user.id, "priority": member_post.name_sv}, headers=auth_headers(member_token), ) @@ -161,7 +161,7 @@ def test_update_signup_without_changing_group_name_is_allowed( """Not sending a group name with the update body should make it stay as-is""" signup = client.post( f"/event-signup/{nollning_event['id']}", - json={"user_id": membered_user.id, "group_name": mentor_group.name}, + json={"user_id": membered_user.id, "group_name": mentor_group.name, "priority": "Nolla"}, headers=auth_headers(member_token), ) assert signup.status_code in (200, 201), signup.text @@ -185,7 +185,7 @@ def test_nollning_event_update_signup_remove_group_name_is_disallowed( """Sending a null (or empty) group name to remove the group is disallowed for nollning events""" signup = client.post( f"/event-signup/{nollning_event['id']}", - json={"user_id": membered_user.id, "group_name": mentor_group.name}, + json={"user_id": membered_user.id, "group_name": mentor_group.name, "priority": "Nolla"}, headers=auth_headers(member_token), ) assert signup.status_code in (200, 201), signup.text @@ -247,13 +247,16 @@ def test_update_signup_without_priority_keeps_priority( def test_update_signup_with_null_priority_resets_to_default( - client, member_token, membered_user, nollning_event, mentor_group + client, admin_token, member_token, membered_user, nollning_event ): - """Sending a null priority should reset it to the default one""" + """Sending a null priority should reset it to the default one, for a user who does not match + any of the event's priorities and may therefore hold the default one. + This should only happen if a user loses their role, or is added to the + event by an admin (as done here).""" signup = client.post( f"/event-signup/{nollning_event['id']}", - json={"user_id": membered_user.id, "group_name": mentor_group.name, "priority": "Nolla"}, - headers=auth_headers(member_token), + json={"user_id": membered_user.id, "priority": "Nolla"}, + headers=auth_headers(admin_token), ) assert signup.status_code in (200, 201), signup.text assert signup.json()["priority"] == "Nolla" @@ -266,7 +269,6 @@ def test_update_signup_with_null_priority_resets_to_default( assert response.status_code == 200, response.text assert response.json()["priority"] == DEFAULT_USER_PRIORITY - assert response.json()["group_name"] == mentor_group.name def test_signup_with_priority_the_user_does_not_have(client, member_token, membered_user, nollning_event, mentor_group): @@ -339,7 +341,7 @@ def test_update_signup_to_disallowed_group( """Switching to a group of a disallowed type is rejected, and nothing else is changed.""" signup = client.post( f"/event-signup/{nollning_event['id']}", - json={"user_id": membered_user.id, "group_name": mentor_group.name}, + json={"user_id": membered_user.id, "group_name": mentor_group.name, "priority": "Nolla"}, headers=auth_headers(member_token), ) assert signup.status_code in (200, 201), signup.text @@ -355,4 +357,139 @@ def test_update_signup_to_disallowed_group( f"/event-signup/me-signup/{nollning_event['id']}", headers=auth_headers(member_token) ).json() assert signup_after["group_name"] == mentor_group.name - assert signup_after["priority"] != "Nolla" + + +def test_signup_with_default_priority_when_matching_a_priority_is_blocked( + client, member_token, membered_user, nollning_event, mentor_group +): + """A nolla who signs up as "Övrigt" is confusing, so we reject it.""" + response = client.post( + f"/event-signup/{nollning_event['id']}", + json={"user_id": membered_user.id, "group_name": mentor_group.name, "priority": DEFAULT_USER_PRIORITY}, + headers=auth_headers(member_token), + ) + + assert response.status_code == 403, response.text + + +@pytest.mark.parametrize("sent_priority", [None, ""]) +def test_signup_without_priority_when_matching_a_priority_is_blocked( + client, member_token, membered_user, nollning_event, mentor_group, sent_priority +): + """Leaving the priority out is the same as sending "Övrigt", and is rejected the same way.""" + body = {"user_id": membered_user.id, "group_name": mentor_group.name} + if sent_priority is not None: + body["priority"] = sent_priority + + response = client.post( + f"/event-signup/{nollning_event['id']}", + json=body, + headers=auth_headers(member_token), + ) + + assert response.status_code == 403, response.text + + +def test_signup_with_matching_priority_is_still_allowed( + client, member_token, membered_user, nollning_event, mentor_group +): + """The priority the user actually holds is of course still accepted.""" + response = client.post( + f"/event-signup/{nollning_event['id']}", + json={"user_id": membered_user.id, "group_name": mentor_group.name, "priority": "Nolla"}, + headers=auth_headers(member_token), + ) + + assert response.status_code in (200, 201), response.text + assert response.json()["priority"] == "Nolla" + + +def test_signup_with_default_priority_without_matching_a_priority_is_allowed( + client, member_token, membered_user, admin_token, admin_council_id +): + """Someone who matches none of the event's priorities keeps the default one.""" + data = event_data_factory(council_id=admin_council_id, priorities=["Nolla"]) + event = client.post("/events/", json=data, headers=auth_headers(admin_token)).json() + + response = client.post( + f"/event-signup/{event['id']}", + json={"user_id": membered_user.id, "priority": DEFAULT_USER_PRIORITY}, + headers=auth_headers(member_token), + ) + + assert response.status_code in (200, 201), response.text + assert response.json()["priority"] == DEFAULT_USER_PRIORITY + + +def test_signup_with_default_priority_on_event_without_priorities_is_allowed( + client, + member_token, + membered_user, + event, + mentor_group, # Member has a nolla priority, but that doesn't matter since the event has no priorities +): + """An event which asks for no priorities cannot be matched, so the default one is fine.""" + response = client.post( + f"/event-signup/{event['id']}", + json={"user_id": membered_user.id, "priority": DEFAULT_USER_PRIORITY}, + headers=auth_headers(member_token), + ) + + assert response.status_code in (200, 201), response.text + assert response.json()["priority"] == DEFAULT_USER_PRIORITY + + +def test_signup_with_default_priority_when_matching_a_post_priority_is_blocked( + client, member_token, membered_user, member_post, admin_token, admin_council_id +): + """Post priorities count as a match just like the nollning ones do. + We reject a signup with the default priority if the user has a post which + matches one of the event's priorities.""" + data = event_data_factory(council_id=admin_council_id, priorities=[member_post.name_sv]) + event = client.post("/events/", json=data, headers=auth_headers(admin_token)).json() + + response = client.post( + f"/event-signup/{event['id']}", + json={"user_id": membered_user.id, "priority": DEFAULT_USER_PRIORITY}, + headers=auth_headers(member_token), + ) + + assert response.status_code == 403, response.text + + +def test_admin_can_sign_up_matching_user_with_default_priority( + client, admin_token, membered_user, nollning_event, mentor_group +): + """Admins make the final call and are not restricted by the priorities.""" + response = client.post( + f"/event-signup/{nollning_event['id']}", + json={"user_id": membered_user.id, "group_name": mentor_group.name, "priority": DEFAULT_USER_PRIORITY}, + headers=auth_headers(admin_token), + ) + + assert response.status_code in (200, 201), response.text + assert response.json()["priority"] == DEFAULT_USER_PRIORITY + + +def test_update_signup_to_default_priority_when_matching_a_priority_is_blocked( + client, member_token, membered_user, nollning_event, mentor_group +): + """The same rule holds when editing an existing signup.""" + signup = client.post( + f"/event-signup/{nollning_event['id']}", + json={"user_id": membered_user.id, "group_name": mentor_group.name, "priority": "Nolla"}, + headers=auth_headers(member_token), + ) + assert signup.status_code in (200, 201), signup.text + + response = client.patch( + f"/event-signup/{nollning_event['id']}", + json={"priority": DEFAULT_USER_PRIORITY}, + headers=auth_headers(member_token), + ) + + assert response.status_code == 403, response.text + signup_after = client.get( + f"/event-signup/me-signup/{nollning_event['id']}", headers=auth_headers(member_token) + ).json() + assert signup_after["priority"] == "Nolla" diff --git a/tests/test_events.py b/tests/test_events.py index c1523a13..c3aebb9f 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -1,6 +1,13 @@ # type: ignore import pytest -from .basic_factories import add_user_to_group, auth_headers, event_data_factory +from datetime import datetime, timedelta, timezone +from helpers.constants import DEFAULT_USER_PRIORITY +from .basic_factories import ( + add_user_to_group, + auth_headers, + create_membered_user, + event_data_factory, +) class TestCreateEvent: @@ -155,3 +162,111 @@ def test_delete_event_forbidden(self, client, member_token, event): response = client.delete(f"/events/{event['id']}", headers=auth_headers(member_token)) assert response.status_code == 403 + + +def _close_signup(db_session, event_id): + """Move the signup deadline into the past so that spots may be handed out.""" + from db_models.event_model import Event_DB + + event = db_session.query(Event_DB).filter_by(id=event_id).one() + event.signup_end = datetime.now(timezone.utc) - timedelta(minutes=1) + db_session.commit() + + return event + + +def _signup_users(client, db_session, admin_token, event_id, count, priority, email_prefix): + """Sign `count` fresh users up to the event as an admin, which bypasses the signup checks.""" + users = [] + for i in range(count): + user = create_membered_user(client, db_session, email=f"{email_prefix}{i}@example.com") + response = client.post( + f"/event-signup/{event_id}", + json={"user_id": user.id, "priority": priority}, + headers=auth_headers(admin_token), + ) + assert response.status_code in (200, 201), response.text + users.append(user) + + return users + + +def _hand_out_spots(client, admin_token, event_id): + return client.post(f"/events/event-signups/{event_id}", headers=auth_headers(admin_token)) + + +def _confirmed_signups(client, admin_token, event_id): + response = client.get(f"/events/event-signups/all/{event_id}", headers=auth_headers(admin_token)) + assert response.status_code == 200, response.text + + return [signup for signup in response.json() if signup["confirmed_status"]] + + +class TestHandOutSpots: + """Test POST /events/event-signups/{event_id}, the "dela ut platser" endpoint""" + + def test_prioritized_people_do_not_exceed_max_event_users(self, client, db_session, admin_token, admin_council_id): + """More prioritized signups than seats must not confirm more people than there are seats.""" + data = event_data_factory(council_id=admin_council_id, max_event_users=2, priorities=["Nolla"]) + event = client.post("/events/", json=data, headers=auth_headers(admin_token)).json() + + _signup_users(client, db_session, admin_token, event["id"], 3, "Nolla", "nolla") + _close_signup(db_session, event["id"]) + + response = _hand_out_spots(client, admin_token, event["id"]) + + assert response.status_code in (200, 201), response.text + assert len(response.json()) == 2 + assert len(_confirmed_signups(client, admin_token, event["id"])) == 2 + + def test_oversubscribed_priority_does_not_let_in_non_prioritized_people( + self, client, db_session, admin_token, admin_council_id + ): + """The negative `places_left` must not be used as a slice, which would admit extra people.""" + data = event_data_factory(council_id=admin_council_id, max_event_users=2, priorities=["Nolla"]) + event = client.post("/events/", json=data, headers=auth_headers(admin_token)).json() + + _signup_users(client, db_session, admin_token, event["id"], 3, "Nolla", "nolla") + _signup_users(client, db_session, admin_token, event["id"], 3, DEFAULT_USER_PRIORITY, "ovrig") + _close_signup(db_session, event["id"]) + + response = _hand_out_spots(client, admin_token, event["id"]) + + assert response.status_code in (200, 201), response.text + confirmed = _confirmed_signups(client, admin_token, event["id"]) + assert len(confirmed) == 2 + assert all(signup["priority"] == "Nolla" for signup in confirmed) + + def test_lottery_event_also_stops_at_max_event_users(self, client, db_session, admin_token, admin_council_id): + """The lottery branch is capped just like the FIFO one.""" + data = event_data_factory(council_id=admin_council_id, max_event_users=2, priorities=["Nolla"], lottery=True) + event = client.post("/events/", json=data, headers=auth_headers(admin_token)).json() + + _signup_users(client, db_session, admin_token, event["id"], 4, "Nolla", "nolla") + _signup_users(client, db_session, admin_token, event["id"], 2, DEFAULT_USER_PRIORITY, "ovrig") + _close_signup(db_session, event["id"]) + + response = _hand_out_spots(client, admin_token, event["id"]) + + assert response.status_code in (200, 201), response.text + confirmed = _confirmed_signups(client, admin_token, event["id"]) + assert len(confirmed) == 2 + assert all(signup["priority"] == "Nolla" for signup in confirmed) + + def test_places_left_are_filled_with_non_prioritized_people( + self, client, db_session, admin_token, admin_council_id + ): + """Seats the prioritized people do not use are still handed out to everyone else.""" + data = event_data_factory(council_id=admin_council_id, max_event_users=3, priorities=["Nolla"]) + event = client.post("/events/", json=data, headers=auth_headers(admin_token)).json() + + prioritized = _signup_users(client, db_session, admin_token, event["id"], 1, "Nolla", "nolla") + _signup_users(client, db_session, admin_token, event["id"], 4, DEFAULT_USER_PRIORITY, "ovrig") + _close_signup(db_session, event["id"]) + + response = _hand_out_spots(client, admin_token, event["id"]) + + assert response.status_code in (200, 201), response.text + confirmed = _confirmed_signups(client, admin_token, event["id"]) + assert len(confirmed) == 3 + assert prioritized[0].id in {signup["user"]["id"] for signup in confirmed}