Fix none nollning group test fails

api_schemas/event_signup_schemas.py

@@ -21,6 +21,8 @@ class EventSignupRead(BaseSchema):
 
 
 class EventSignupUpdate(BaseSchema):
+    """This schema uses partial updates, any field not sent to backend will be ignored and not updated."""
+
     user_id: int | None = None
     priority: str | None = None
     group_name: str | None = None

helpers/constants.py

@@ -96,6 +96,8 @@
 
 # Event User
 DEFAULT_USER_PRIORITY = "Övrigt"
+# Priorities tied to a nollning group instead of a post
+NOLLNING_PRIORITIES = ["Nolla", "Gruppfadder", "Uppdragsfadder"]
 
 # Guild Meeting
 MAX_GUILD_MEETING_DATE_DESC = 500

services/event_signup_service.py

@@ -8,10 +8,21 @@
 from db_models.group_model import Group_DB
 from db_models.group_user_model import GroupUser_DB
 from api_schemas.event_signup_schemas import EventSignupCreate, EventSignupUpdate
-from helpers.constants import DEFAULT_USER_PRIORITY
+from helpers.constants import DEFAULT_USER_PRIORITY, NOLLNING_PRIORITIES
 from helpers.types import GROUP_TYPE
 
 
+def user_matches_existing_event_post_priorities(user: User_DB, event: Event_DB):
+    """Only true if a user has a post which matches one of the event's priorities,
+    and that priority match is not a nollning priority (which are tied to groups instead of posts)."""
+    if not event.priorities:
+        return False
+
+    event_post_priorities = {p.priority for p in event.priorities if p.priority not in NOLLNING_PRIORITIES}
+
+    return any(post.name_sv in event_post_priorities for post in user.posts)
+
+
 def signup_to_event(event: Event_DB, user: User_DB, data: EventSignupCreate, manage_permission: bool, db: Session):
     now = datetime.now(UTC)
 
@@ -83,18 +94,30 @@ def update_event_signup(event: Event_DB, data: EventSignupUpdate, user_id: int,
     if signup is None:
         raise HTTPException(status.HTTP_404_NOT_FOUND)
 
-    if manage_permission == False and not is_group_allowed(
-        event, db.query(User_DB).filter(User_DB.id == user_id).one(), data.group_name
+    # Only fields the client actually sent are considered.
+    updates = data.model_dump(exclude_unset=True, exclude={"user_id"})
+
+    if "group_name" in updates and not updates["group_name"]:  # if falsy
+        updates["group_name"] = None
+
+    # Only authorize the group when the client asked to change it, otherwise an update that leaves
+    # the group alone would be rejected on nollning events for not carrying a group at all.
+    if (
+        manage_permission == False
+        and "group_name" in updates
+        and not is_group_allowed(event, db.query(User_DB).filter(User_DB.id == user_id).one(), updates["group_name"])
     ):
         raise HTTPException(status.HTTP_403_FORBIDDEN, detail="User cannot sign up with this group")
 
-    for var, value in vars(data).items():
-        if var == "priority" and not value:
-            setattr(signup, "priority", DEFAULT_USER_PRIORITY)
-        elif var == "group_name" and not value:
-            setattr(signup, "group_name", None)
-        else:
-            setattr(signup, var, value) if value else None
+    # priority and drinkPackage are not nullable in the database, so a null means "back to default"
+    if "priority" in updates and not updates["priority"]:  # if falsy
+        updates["priority"] = DEFAULT_USER_PRIORITY
+
+    if "drinkPackage" in updates and updates["drinkPackage"] is None:
+        del updates["drinkPackage"]  # None is not a valid value ("None" is), so leave the old value in place
+
+    for var, value in updates.items():
+        setattr(signup, var, value)
 
     if not event.drink_package:
         signup.drinkPackage = "None"
@@ -132,7 +155,9 @@ def get_allowed_groups(event: Event_DB, user: User_DB):
 def is_group_allowed(event: Event_DB, user: User_DB, group_name: str | None):
     if event.is_nollning_event:
         if group_name is None:
-            return False
+            # Without a group the user has to qualify through a post priority instead,
+            # since nollning priorities are tied to a group
+            return user_matches_existing_event_post_priorities(user, event)
 
         allowed_group_types = event.mentor_group_types or list(get_args(GROUP_TYPE))
         is_event_allowed = False

tests/test_event_signup.py

@@ -1,5 +1,6 @@
 # type: ignore
 import pytest
+from helpers.constants import DEFAULT_USER_PRIORITY
 from .basic_factories import add_user_to_group, auth_headers, event_data_factory
 
 
@@ -37,14 +38,47 @@ def test_signup_with_group_the_user_is_not_in(client, member_token, membered_use
     assert response.status_code == 403
 
 
-def test_signup_without_group(client, member_token, membered_user, nollning_event, mission_group):
-    """Signing up without picking a group is not restricted by the group types."""
+def test_nollning_event_signup_without_group(client, member_token, membered_user, nollning_event, mission_group):
+    """Signing up to nollning event without picking a group is not allowed."""
     response = client.post(
         f"/event-signup/{nollning_event['id']}",
         json={"user_id": membered_user.id},
         headers=auth_headers(member_token),
     )
 
+    assert response.status_code == 403, response.text
+
+
+def test_nollning_event_signup_without_group_with_post_priority(
+    client, admin_token, admin_council_id, member_token, membered_user, member_post
+):
+    """A user whose post matches one of the event's priorities may sign up without a group."""
+    data = event_data_factory(
+        council_id=admin_council_id,
+        is_nollning_event=True,
+        mentor_group_types=["Mentor"],
+        priorities=[member_post.name_sv, "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},
+        headers=auth_headers(member_token),
+    )
+
+    assert response.status_code in (200, 201), response.text
+    assert response.json()["group_name"] is None
+
+
+def test_non_nollning_event_signup_without_group(client, member_token, membered_user, event, mission_group):
+    """Signing up to non-nollning event without picking a group is not restricted by the group types."""
+    response = client.post(
+        f"/event-signup/{event['id']}",
+        json={"user_id": membered_user.id},
+        headers=auth_headers(member_token),
+    )
+
     assert response.status_code in (200, 201), response.text
     assert response.json()["group_name"] is None
 
@@ -106,25 +140,119 @@ def test_non_nollning_event_ignores_group_types(client, member_token, membered_u
     assert response.status_code in (200, 201), response.text
 
 
-def test_update_signup_without_group_name_is_allowed(client, member_token, membered_user, nollning_event, mentor_group):
-    """
-    Regression: patching some other field of a signup on a nollning event used to be
-    rejected with 403, because an omitted group_name was checked as if it were a group
-    the user isn't in.
-    """
+def test_update_signup_without_changing_group_name_is_allowed(
+    client, member_token, membered_user, nollning_event, mentor_group
+):
+    """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},
+        headers=auth_headers(member_token),
+    )
+    assert signup.status_code in (200, 201), signup.text
+    assert signup.json()["group_name"] == mentor_group.name
+
+    response = client.patch(
+        f"/event-signup/{nollning_event['id']}",
+        json={"priority": "Nolla"},
+        headers=auth_headers(member_token),
+    )
+
+    assert response.status_code == 200, response.text
+    assert response.json()["priority"] == "Nolla"
+    assert response.json()["group_name"] == mentor_group.name
+
+
+@pytest.mark.parametrize("sent_group_name", [None, ""])
+def test_nollning_event_update_signup_remove_group_name_is_disallowed(
+    client, member_token, membered_user, nollning_event, mentor_group, sent_group_name
+):
+    """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},
         headers=auth_headers(member_token),
     )
     assert signup.status_code in (200, 201), signup.text
+    assert signup.json()["group_name"] == mentor_group.name
+
+    response = client.patch(
+        f"/event-signup/{nollning_event['id']}",
+        json={"group_name": sent_group_name},
+        headers=auth_headers(member_token),
+    )
+
+    assert response.status_code == 403, response.text
+
+
+@pytest.mark.parametrize("sent_group_name", [None, ""])
+def test_non_nollning_event_update_signup_remove_group_name_is_allowed(
+    client, member_token, membered_user, event, mentor_group, sent_group_name
+):
+    """Sending a null (or empty) group name to remove the group is allowed for non-nollning events"""
+    signup = client.post(
+        f"/event-signup/{event['id']}",
+        json={"user_id": membered_user.id, "group_name": mentor_group.name},
+        headers=auth_headers(member_token),
+    )
+    assert signup.status_code in (200, 201), signup.text
+    assert signup.json()["group_name"] == mentor_group.name
+
+    response = client.patch(
+        f"/event-signup/{event['id']}",
+        json={"priority": "Nolla", "group_name": sent_group_name},
+        headers=auth_headers(member_token),
+    )
+
+    assert response.status_code == 200, response.text
+    assert response.json()["priority"] == "Nolla"
+    assert response.json()["group_name"] is None
+
+
+def test_update_signup_without_priority_keeps_priority(
+    client, member_token, membered_user, nollning_event, mentor_group
+):
+    """Leaving priority out of 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, "priority": "Nolla"},
+        headers=auth_headers(member_token),
+    )
+    assert signup.status_code in (200, 201), signup.text
+    assert signup.json()["priority"] == "Nolla"
 
     response = client.patch(
-        f"/event-signup/{nollning_event['id']}", json={"priority": "Nolla"}, headers=auth_headers(member_token)
+        f"/event-signup/{nollning_event['id']}",
+        json={"drinkPackage": "Alcohol"},
+        headers=auth_headers(member_token),
     )
 
     assert response.status_code == 200, response.text
     assert response.json()["priority"] == "Nolla"
+    assert response.json()["group_name"] == mentor_group.name
+
+
+def test_update_signup_with_null_priority_resets_to_default(
+    client, member_token, membered_user, nollning_event, mentor_group
+):
+    """Sending a null priority should reset it to the default one"""
+    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
+    assert signup.json()["priority"] == "Nolla"
+
+    response = client.patch(
+        f"/event-signup/{nollning_event['id']}",
+        json={"priority": None},
+        headers=auth_headers(member_token),
+    )
+
+    assert response.status_code == 200, response.text
+    assert response.json()["priority"] == DEFAULT_USER_PRIORITY
+    assert response.json()["group_name"] == mentor_group.name
 
 
 def test_update_signup_to_disallowed_group(