Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions events/schema.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
import logging
from copy import deepcopy
from datetime import timedelta
from typing import List, Optional

import graphene
from auditlog_extra.graphene_decorators import auditlog_access
from django.apps import apps
from django.conf import settings
from django.core.exceptions import PermissionDenied, ValidationError
from django.db import transaction
from django.db.models import Count, Prefetch, Q
from django.utils import timezone
from django.utils.translation import get_language
from graphene import Connection, ObjectType, relay
from graphene_django import DjangoObjectType
Expand Down Expand Up @@ -49,6 +52,7 @@
TicketSystemPasswordAlreadyAssignedError,
TicketSystemPasswordNothingToImportError,
TicketVerificationError,
TooLateToUnenrolError,
)
from kukkuu.utils import get_kukkuu_error_by_code
from projects.models import Project
Expand All @@ -61,19 +65,33 @@
EventGroupTranslation = apps.get_model("events", "EventGroupTranslation")


def validate_enrolment(child: Child, occurrence: Occurrence):
def validate_enrolment(child: Child, occurrence: Occurrence) -> None:
if reason := occurrence.get_enrolment_denied_reason(child):
raise ENROLMENT_DENIED_REASON_TO_GRAPHQL_ERROR[reason]


def validate_enrolment_deletion(enrolment):
def validate_enrolment_deletion(enrolment: Enrolment) -> None:
now = timezone.now()
occurrence_time = enrolment.occurrence.time

# Check if the enrolment is for an upcoming occurrence.
# User should never be able to unenrol, if the occurrence is in the past.
if not enrolment.is_upcoming():
raise PastEnrolmentError(
"Cannot unenrol from an occurrence that is in the past."
)

# Get the unenrol hours limit from settings, 0 means no time limit
# (only past occurrences are not allowed to unenrol).
hours_before = settings.KUKKUU_ENROLMENT_UNENROL_HOURS_BEFORE
if hours_before > 0 and (occurrence_time - now) <= timedelta(hours=hours_before):
raise TooLateToUnenrolError(
"Cannot unenrol from an occurrence that starts within "
f"{hours_before} hours."
)


def validate_occurrence_input(kwargs, occurrence: Occurrence = None):
def validate_occurrence_input(kwargs, occurrence: Occurrence = None) -> None:
if time := kwargs.get("time"):
if occurrence:
# Don't consider the occurrence which is being updated
Expand Down
49 changes: 48 additions & 1 deletion events/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -979,7 +979,9 @@ def test_unenrol_occurrence(
occurrence,
project,
child_with_random_guardian,
settings,
):
settings.KUKKUU_ENROLMENT_UNENROL_HOURS_BEFORE = 0
non_authen_executed = api_client.execute(
UNENROL_OCCURRENCE_MUTATION, variables=ENROL_OCCURRENCE_VARIABLES
)
Expand Down Expand Up @@ -1021,7 +1023,8 @@ def test_unenrol_occurrence(
snapshot.assert_match(executed)


def test_ticket_not_valid_after_unenrol(user_api_client, occurrence, project):
def test_ticket_not_valid_after_unenrol(user_api_client, occurrence, project, settings):
settings.KUKKUU_ENROLMENT_UNENROL_HOURS_BEFORE = 0
child = ChildWithGuardianFactory(
relationship__guardian__user=user_api_client.user, project=project
)
Expand Down Expand Up @@ -1059,6 +1062,50 @@ def test_cannot_unenrol_from_occurrence_in_past(
assert_match_error_code(executed, PAST_ENROLMENT_ERROR)


@freeze_time("2024-01-01 12:00:00")
def test_cannot_unenrol_within_48_hours(
guardian_api_client, child_with_user_guardian, settings
):
settings.KUKKUU_ENROLMENT_UNENROL_HOURS_BEFORE = 48
# Create occurrence 47 hours in future (within 48 hour limit)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about setting the explicit value for the environment variable here so the test is robust against local .env file changes?

Suggested change
# Create occurrence 47 hours in future (within 48 hour limit)
settings.KUKKUU_ENROLMENT_UNENROL_HOURS_BEFORE = 48
# Create occurrence 47 hours in future (within 48 hour limit)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without it, it's testing the default behaviour and that's what I think we should use. I have not added the environment variable to pipeline configurations at all, so I'm trusting in default value.

@karisal-anders karisal-anders Jun 2, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the tests assume that the value is always set to 48 then how about adding a simple test case for that? I mean a test case just for testing that KUKKUU_ENROLMENT_UNENROL_HOURS_BEFORE is 48. Otherwise there is a hidden dependency that one can not override the environment variable to any other value in any environment when running the tests or these cases will fail. They will do so with the added test case also though, but at least then the dependency on the value being 48 in environment would be documented by it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added settings.KUKKUU_ENROLMENT_UNENROL_HOURS_BEFORE = 48

future_time = timezone.now() + timedelta(hours=47)
enrolment = EnrolmentFactory(
occurrence__time=future_time, child=child_with_user_guardian
)

variables = deepcopy(UNENROL_OCCURRENCE_VARIABLES)
variables["input"]["occurrenceId"] = get_global_id(enrolment.occurrence)
variables["input"]["childId"] = get_global_id(child_with_user_guardian)

executed = guardian_api_client.execute(
UNENROL_OCCURRENCE_MUTATION, variables=variables
)

assert_match_error_code(executed, "TOO_LATE_TO_UNENROL_ERROR")


@freeze_time("2024-01-01 12:00:00")
def test_can_unenrol_outside_48_hours(
guardian_api_client, child_with_user_guardian, settings
):
settings.KUKKUU_ENROLMENT_UNENROL_HOURS_BEFORE = 48
# Create occurrence 49 hours in future (outside 48 hour limit)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about setting the explicit value for the environment variable here so the test is robust against local .env file changes?

Suggested change
# Create occurrence 49 hours in future (outside 48 hour limit)
settings.KUKKUU_ENROLMENT_UNENROL_HOURS_BEFORE = 48
# Create occurrence 49 hours in future (outside 48 hour limit)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikomakela nikomakela Jun 3, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added settings.KUKKUU_ENROLMENT_UNENROL_HOURS_BEFORE = 48

future_time = timezone.now() + timedelta(hours=49)
enrolment = EnrolmentFactory(
occurrence__time=future_time, child=child_with_user_guardian
)

variables = deepcopy(UNENROL_OCCURRENCE_VARIABLES)
variables["input"]["occurrenceId"] = get_global_id(enrolment.occurrence)
variables["input"]["childId"] = get_global_id(child_with_user_guardian)

executed = guardian_api_client.execute(
UNENROL_OCCURRENCE_MUTATION, variables=variables
)

assert "errors" not in executed


def test_maximum_enrolment(
guardian_api_client, occurrence, project, child_with_user_guardian
):
Expand Down
2 changes: 2 additions & 0 deletions events/tests/test_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,9 @@ def test_unenrol_occurrence_notification(
occurrence,
notification_template_occurrence_unenrolment_fi,
mock_user_create_subscriptions_management_auth_token,
settings,
):
settings.KUKKUU_ENROLMENT_UNENROL_HOURS_BEFORE = 0
child = ChildWithGuardianFactory(
relationship__guardian__user=guardian_api_client.user,
project=project,
Expand Down
1 change: 1 addition & 0 deletions kukkuu/consts.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
OCCURRENCE_IS_NOT_FULL_ERROR = "OCCURRENCE_IS_NOT_FULL_ERROR"
MESSAGE_ALREADY_SENT_ERROR = "MESSAGE_ALREADY_SENT_ERROR"
PAST_ENROLMENT_ERROR = "PAST_ENROLMENT_ERROR"
TOO_LATE_TO_UNENROL_ERROR = "TOO_LATE_TO_UNENROL_ERROR"
SINGLE_EVENTS_DISALLOWED_ERROR = "SINGLE_EVENTS_DISALLOWED_ERROR"
TICKET_SYSTEM_URL_MISSING_ERROR = "TICKET_SYSTEM_URL_MISSING_ERROR"
NO_FREE_TICKET_SYSTEM_PASSWORDS_ERROR = "NO_FREE_TICKET_SYSTEM_PASSWORDS_ERROR"
Expand Down
4 changes: 4 additions & 0 deletions kukkuu/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ class PastEnrolmentError(KukkuuGraphQLError):
"""Cannot unenrol because the enrolment is in the past"""


class TooLateToUnenrolError(KukkuuGraphQLError):
"""Cannot unenrol because the occurrence starts too soon"""


class SingleEventsDisallowedError(KukkuuGraphQLError):
"""Cannot create an event outside event groups"""

Expand Down
5 changes: 5 additions & 0 deletions kukkuu/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
KUKKUU_REMINDER_DAYS_IN_ADVANCE=(int, 1),
KUKKUU_FEEDBACK_NOTIFICATION_DELAY=(int, 15),
KUKKUU_NOTIFICATIONS_SHEET_ID=(str, ""),
KUKKUU_ENROLMENT_UNENROL_HOURS_BEFORE=(int, 48),
LOGIN_REDIRECT_URL=(str, "/admin/"),
LOGOUT_REDIRECT_URL=(str, "/admin/"),
VERIFICATION_TOKEN_VALID_MINUTES=(int, 15),
Expand Down Expand Up @@ -471,6 +472,10 @@ def sentry_before_send(event: Event, hint: Hint):
KUKKUU_REMINDER_DAYS_IN_ADVANCE = env("KUKKUU_REMINDER_DAYS_IN_ADVANCE")
KUKKUU_FEEDBACK_NOTIFICATION_DELAY = env("KUKKUU_FEEDBACK_NOTIFICATION_DELAY")
KUKKUU_NOTIFICATIONS_SHEET_ID = env("KUKKUU_NOTIFICATIONS_SHEET_ID")
# How many hours before the occurrence start time
# the user can unenrol from the occurrence.
# If set to 0, unenrolment is allowed till the occurrence start.
KUKKUU_ENROLMENT_UNENROL_HOURS_BEFORE = env("KUKKUU_ENROLMENT_UNENROL_HOURS_BEFORE")

KUKKUU_HASHID_MIN_LENGTH = 5
KUKKUU_HASHID_ALPHABET = "abcdefghijklmnopqrstuvwxyz"
Expand Down
3 changes: 3 additions & 0 deletions kukkuu/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
TICKET_SYSTEM_PASSWORD_ALREADY_ASSIGNED_ERROR,
TICKET_SYSTEM_PASSWORD_NOTHING_TO_IMPORT_ERROR,
TICKET_SYSTEM_URL_MISSING_ERROR,
TOO_LATE_TO_UNENROL_ERROR,
VERIFICATION_TOKEN_INVALID_ERROR,
)
from kukkuu.exceptions import (
Expand Down Expand Up @@ -67,6 +68,7 @@
TicketSystemPasswordAlreadyAssignedError,
TicketSystemPasswordNothingToImportError,
TicketSystemUrlMissingError,
TooLateToUnenrolError,
VerificationTokenInvalidError,
)

Expand Down Expand Up @@ -97,6 +99,7 @@
EventGroupNotReadyForPublishingError: EVENT_GROUP_NOT_READY_FOR_PUBLISHING_ERROR,
EventNotPublishedError: EVENT_NOT_PUBLISHED_ERROR,
PastEnrolmentError: PAST_ENROLMENT_ERROR,
TooLateToUnenrolError: TOO_LATE_TO_UNENROL_ERROR,
SingleEventsDisallowedError: SINGLE_EVENTS_DISALLOWED_ERROR,
TicketSystemUrlMissingError: TICKET_SYSTEM_URL_MISSING_ERROR,
NoFreeTicketSystemPasswordsError: NO_FREE_TICKET_SYSTEM_PASSWORDS_ERROR,
Expand Down