Fix signup issues

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()

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)
 

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"