From c34f9799e787f676fa2ced6d3ccdfdaf1421850d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Bl=C3=A5man?= Date: Fri, 7 Aug 2026 15:42:27 +0000 Subject: [PATCH 1/6] Fix none nollning group test fails --- services/event_signup_service.py | 6 ++++-- tests/test_event_signup.py | 14 +++++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/services/event_signup_service.py b/services/event_signup_service.py index f0d2363c..69605b31 100644 --- a/services/event_signup_service.py +++ b/services/event_signup_service.py @@ -83,8 +83,10 @@ 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 + if ( + manage_permission == False + and data.group_name is not None + and not is_group_allowed(event, db.query(User_DB).filter(User_DB.id == user_id).one(), data.group_name) ): raise HTTPException(status.HTTP_403_FORBIDDEN, detail="User cannot sign up with this group") diff --git a/tests/test_event_signup.py b/tests/test_event_signup.py index 97be0916..568e51e9 100644 --- a/tests/test_event_signup.py +++ b/tests/test_event_signup.py @@ -37,7 +37,7 @@ 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): +def test_nollning_event_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.""" response = client.post( f"/event-signup/{nollning_event['id']}", @@ -45,6 +45,18 @@ def test_signup_without_group(client, member_token, membered_user, nollning_even headers=auth_headers(member_token), ) + assert response.status_code == 403, 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 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 From 9238a8dd5dcd4e6a3c348822276a5e77ca81fcfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Bl=C3=A5man?= Date: Fri, 7 Aug 2026 15:49:43 +0000 Subject: [PATCH 2/6] Fixed doc comments on tests --- tests/test_event_signup.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_event_signup.py b/tests/test_event_signup.py index 568e51e9..c62d710b 100644 --- a/tests/test_event_signup.py +++ b/tests/test_event_signup.py @@ -38,7 +38,7 @@ def test_signup_with_group_the_user_is_not_in(client, member_token, membered_use def test_nollning_event_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.""" + """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}, @@ -46,11 +46,10 @@ def test_nollning_event_signup_without_group(client, member_token, membered_user ) assert response.status_code == 403, 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 without picking a group is not restricted by the group types.""" + """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}, From 9ebea233906a83f76c9ac6d6c2ac184db9509c72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Bl=C3=A5man?= Date: Mon, 10 Aug 2026 18:15:54 +0000 Subject: [PATCH 3/6] Changed event signup update so that group None => no change, and added more tests --- services/event_signup_service.py | 4 +-- tests/test_event_signup.py | 60 ++++++++++++++++++++++++++++---- 2 files changed, 55 insertions(+), 9 deletions(-) diff --git a/services/event_signup_service.py b/services/event_signup_service.py index 69605b31..921f9e4f 100644 --- a/services/event_signup_service.py +++ b/services/event_signup_service.py @@ -91,9 +91,9 @@ def update_event_signup(event: Event_DB, data: EventSignupUpdate, user_id: int, 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: + if var == "priority" and not value and value is not None: # falsy but not None setattr(signup, "priority", DEFAULT_USER_PRIORITY) - elif var == "group_name" and not value: + elif var == "group_name" and not value and value is not None: # falsy but not None setattr(signup, "group_name", None) else: setattr(signup, var, value) if value else None diff --git a/tests/test_event_signup.py b/tests/test_event_signup.py index c62d710b..1a3f8b42 100644 --- a/tests/test_event_signup.py +++ b/tests/test_event_signup.py @@ -117,25 +117,71 @@ 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 update body (or sending null) 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 + + +def test_nollning_event_update_signup_remove_group_name_is_disallowed( + client, member_token, membered_user, nollning_event, mentor_group +): + """Changing a group name to an empty string (removing 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={"priority": "Nolla"}, headers=auth_headers(member_token) + f"/event-signup/{nollning_event['id']}", + json={"group_name": ""}, + headers=auth_headers(member_token), + ) + + assert response.status_code == 403, response.text + + +def test_non_nollning_event_update_signup_remove_group_name_is_allowed( + client, member_token, membered_user, event, mentor_group +): + """Changing a group name to an empty string (removing the group) is allowed for 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": ""}, + 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_to_disallowed_group( From 95cf0d0f74e857ab1764968f0246c50ea9f85dc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Bl=C3=A5man?= Date: Mon, 10 Aug 2026 20:53:47 +0000 Subject: [PATCH 4/6] Comment mishap oops --- tests/test_event_signup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_event_signup.py b/tests/test_event_signup.py index 1a3f8b42..51d8d705 100644 --- a/tests/test_event_signup.py +++ b/tests/test_event_signup.py @@ -164,7 +164,7 @@ def test_nollning_event_update_signup_remove_group_name_is_disallowed( def test_non_nollning_event_update_signup_remove_group_name_is_allowed( client, member_token, membered_user, event, mentor_group ): - """Changing a group name to an empty string (removing the group) is allowed for nollning events""" + """Changing a group name to an empty string (removing 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}, From 5c71f90b9bf27f3fd3380717c71333edb7d8e00b Mon Sep 17 00:00:00 2001 From: georgelgeback Date: Tue, 11 Aug 2026 07:58:07 +0000 Subject: [PATCH 5/6] Change signup edit to use exclude_unset=True --- api_schemas/event_signup_schemas.py | 2 + services/event_signup_service.py | 28 ++++++++----- tests/test_event_signup.py | 63 +++++++++++++++++++++++++---- 3 files changed, 77 insertions(+), 16 deletions(-) diff --git a/api_schemas/event_signup_schemas.py b/api_schemas/event_signup_schemas.py index 640794c3..0db24523 100644 --- a/api_schemas/event_signup_schemas.py +++ b/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 diff --git a/services/event_signup_service.py b/services/event_signup_service.py index 921f9e4f..b78abf87 100644 --- a/services/event_signup_service.py +++ b/services/event_signup_service.py @@ -83,20 +83,30 @@ def update_event_signup(event: Event_DB, data: EventSignupUpdate, user_id: int, if signup is None: raise HTTPException(status.HTTP_404_NOT_FOUND) + # 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 data.group_name is not None - and not is_group_allowed(event, db.query(User_DB).filter(User_DB.id == user_id).one(), data.group_name) + 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 and value is not None: # falsy but not None - setattr(signup, "priority", DEFAULT_USER_PRIORITY) - elif var == "group_name" and not value and value is not None: # falsy but not None - 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" diff --git a/tests/test_event_signup.py b/tests/test_event_signup.py index 51d8d705..26b6aa58 100644 --- a/tests/test_event_signup.py +++ b/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 @@ -120,7 +121,7 @@ def test_non_nollning_event_ignores_group_types(client, member_token, membered_u 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 update body (or sending null) should make it stay as-is""" + """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}, @@ -140,10 +141,11 @@ def test_update_signup_without_changing_group_name_is_allowed( 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 + client, member_token, membered_user, nollning_event, mentor_group, sent_group_name ): - """Changing a group name to an empty string (removing the group) is disallowed for nollning events""" + """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}, @@ -154,17 +156,18 @@ def test_nollning_event_update_signup_remove_group_name_is_disallowed( response = client.patch( f"/event-signup/{nollning_event['id']}", - json={"group_name": ""}, + 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 + client, member_token, membered_user, event, mentor_group, sent_group_name ): - """Changing a group name to an empty string (removing the group) is allowed for non-nollning events""" + """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}, @@ -175,7 +178,7 @@ def test_non_nollning_event_update_signup_remove_group_name_is_allowed( response = client.patch( f"/event-signup/{event['id']}", - json={"priority": "Nolla", "group_name": ""}, + json={"priority": "Nolla", "group_name": sent_group_name}, headers=auth_headers(member_token), ) @@ -184,6 +187,52 @@ def test_non_nollning_event_update_signup_remove_group_name_is_allowed( 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={"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( client, member_token, membered_user, nollning_event, mentor_group, mission_group ): From dd220e85e0b3d9ca2cd87d3904923ff759d79420 Mon Sep 17 00:00:00 2001 From: georgelgeback Date: Tue, 11 Aug 2026 10:22:06 +0000 Subject: [PATCH 6/6] Add support for priority holders to skip group_name checks --- helpers/constants.py | 2 ++ services/event_signup_service.py | 17 +++++++++++++++-- tests/test_event_signup.py | 22 ++++++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/helpers/constants.py b/helpers/constants.py index f95b334e..278435e8 100644 --- a/helpers/constants.py +++ b/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 diff --git a/services/event_signup_service.py b/services/event_signup_service.py index b78abf87..fe6dcdd4 100644 --- a/services/event_signup_service.py +++ b/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) @@ -144,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 diff --git a/tests/test_event_signup.py b/tests/test_event_signup.py index 26b6aa58..d008e56e 100644 --- a/tests/test_event_signup.py +++ b/tests/test_event_signup.py @@ -49,6 +49,28 @@ def test_nollning_event_signup_without_group(client, member_token, membered_user 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(