From b0de55b9a0f9021c29665097e65e8c93cd6e3e54 Mon Sep 17 00:00:00 2001 From: SanttuA Date: Thu, 4 Jan 2024 09:38:38 +0200 Subject: [PATCH] Changed reservable before & after rules for online payments Manually confirmed reservations with online payments are allowed to skip reservable before and after validation. This fixes the issue of changing reservable before/after so that previously made reservations couldn't complete their payments due to these validations. --- resources/api/reservation.py | 21 ++++++----- resources/models/utils.py | 17 ++++++++- resources/tests/test_reservation_api.py | 48 +++++++++++++++++++++++++ resources/tests/test_utils.py | 30 +++++++++++++++- 4 files changed, 105 insertions(+), 11 deletions(-) diff --git a/resources/api/reservation.py b/resources/api/reservation.py index af73d2eca..4db8c6bef 100644 --- a/resources/api/reservation.py +++ b/resources/api/reservation.py @@ -50,7 +50,7 @@ ExtraDataMixin ) -from ..models.utils import dateparser, is_reservation_metadata_or_times_different +from ..models.utils import dateparser, has_reservation_data_changed, is_reservation_metadata_or_times_different from respa.renderers import ResourcesBrowsableAPIRenderer from payments.utils import is_free, get_price @@ -317,14 +317,17 @@ def validate(self, data): request_user.save() if not resource.can_ignore_opening_hours(request_user): - reservable_before = resource.get_reservable_before() - if reservable_before and data['begin'] >= reservable_before: - raise ValidationError(_('The resource is reservable only before %(datetime)s' % - {'datetime': reservable_before})) - reservable_after = resource.get_reservable_after() - if reservable_after and data['begin'] < reservable_after: - raise ValidationError(_('The resource is reservable only after %(datetime)s' % - {'datetime': reservable_after})) + # Customers can update to pay their reservation and can skip this rule check to do so + # When paying occurs like this, data does not change and state is waiting for payment + if data.get('state') != Reservation.READY_FOR_PAYMENT or has_reservation_data_changed(data, reservation): + reservable_before = resource.get_reservable_before() + if reservable_before and data['begin'] >= reservable_before: + raise ValidationError(_('The resource is reservable only before %(datetime)s' % + {'datetime': reservable_before})) + reservable_after = resource.get_reservable_after() + if reservable_after and data['begin'] < reservable_after: + raise ValidationError(_('The resource is reservable only after %(datetime)s' % + {'datetime': reservable_after})) # Check given home municipality is included in resource's home municipality set if 'home_municipality' in data: diff --git a/resources/models/utils.py b/resources/models/utils.py index 69a8c0831..7a3c4f1c2 100644 --- a/resources/models/utils.py +++ b/resources/models/utils.py @@ -206,7 +206,7 @@ def set_title(title, *, headers = [], use_extra_fields = True): else: worksheet.write(row_cursor, col, title, title_format) worksheet.set_column(col, col, header[1]) - + row_cursor += 1 header_format = workbook.add_format({'bold': True}) for column, header in enumerate(headers): @@ -763,3 +763,18 @@ def is_reservation_metadata_or_times_different(old_reservation, new_reservation) return True return False + + +def has_reservation_data_changed(data, instance) -> bool: + """ + Returns True when given data has changes compared to reservation instance + and False when not. + """ + if instance == None: + return False + + for field, value in data.items(): + if getattr(instance, field) != value: + return True + + return False diff --git a/resources/tests/test_reservation_api.py b/resources/tests/test_reservation_api.py index 74aff1e36..93fbd5f5c 100644 --- a/resources/tests/test_reservation_api.py +++ b/resources/tests/test_reservation_api.py @@ -2170,6 +2170,54 @@ def test_admins_can_make_reservations_despite_delay( assert response.status_code == 201, msg.format(201, response.status_code, response.data) +@freeze_time('2115-04-02') +@pytest.mark.django_db +def test_reservation_reservable_before_payment_update(user_api_client, reservation, resource_in_unit): + """ + Tests that when reservation state is ready for payment and update has no changing data i.e. user is trying to + pay the reservation, update can be made even if reservable before would normally prevent it + """ + detail_url = reverse('reservation-detail', kwargs={'pk': reservation.pk}) + + field_1 = ReservationMetadataField.objects.get(field_name='reserver_name') + metadata_set = ReservationMetadataSet.objects.create(name='test_set',) + metadata_set.supported_fields.set([field_1]) + reservation.resource.reservation_metadata_set = metadata_set + reservation.resource.save(update_fields=('reservation_metadata_set',)) + + resource_in_unit.reservable_max_days_in_advance = 5 + resource_in_unit.need_manual_confirmation = True + resource_in_unit.save() + + reservation.begin = timezone.now().replace(hour=12, minute=0, second=0) + datetime.timedelta(days=9) + reservation.end = timezone.now().replace(hour=13, minute=0, second=0) + datetime.timedelta(days=9) + reservation.state = Reservation.READY_FOR_PAYMENT + reservation.save() + + reservation_data = { + 'resource': resource_in_unit.id, + 'begin': reservation.begin, + 'end': reservation.end, + 'state': Reservation.READY_FOR_PAYMENT, + 'reserver_name': 'Test Updater' + } + + # attempt to modify reservation, not pay + response = user_api_client.put(detail_url, data=reservation_data, HTTP_ACCEPT_LANGUAGE='en') + assert response.status_code == 400 + assert_non_field_errors_contain(response, 'The resource is reservable only before') + + # attempt to only update/pay reservation + reservation_data = { + 'resource': resource_in_unit.id, + 'begin': reservation.begin, + 'end': reservation.end, + 'state': Reservation.READY_FOR_PAYMENT + } + response = user_api_client.put(detail_url, data=reservation_data, HTTP_ACCEPT_LANGUAGE='en') + assert response.status_code == 200 + + @pytest.mark.django_db def test_reservation_metadata_set(user_api_client, reservation, list_url, reservation_data): detail_url = reverse('reservation-detail', kwargs={'pk': reservation.pk}) diff --git a/resources/tests/test_utils.py b/resources/tests/test_utils.py index 42f93963d..9218d8303 100644 --- a/resources/tests/test_utils.py +++ b/resources/tests/test_utils.py @@ -3,7 +3,7 @@ from django.conf import settings from django.test.utils import override_settings from resources.models.utils import ( - get_payment_requested_waiting_time, is_reservation_metadata_or_times_different, format_dt_range_alt + get_payment_requested_waiting_time, has_reservation_data_changed, is_reservation_metadata_or_times_different, format_dt_range_alt ) from resources.models import Reservation, Resource, Unit from payments.models import Product, Order @@ -289,3 +289,31 @@ def test_format_dt_range_alt_different_day(reservation_basic): return_value = format_dt_range_alt('en', begin, end) expected_value = '2.2.2022 12:00 – 3.2.2022 14:00' assert return_value == expected_value + + +@pytest.mark.django_db +def test_has_reservation_data_changed_no_changes(reservation_basic): + data = {'reserver_name': reservation_basic.reserver_name} + result = has_reservation_data_changed(data, reservation_basic) + assert result is False + + +@pytest.mark.django_db +def test_has_reservation_data_changed_changes(reservation_basic): + data = {'reserver_name': 'changed name'} + result = has_reservation_data_changed(data, reservation_basic) + assert result is True + + +@pytest.mark.django_db +def test_has_reservation_data_changed_no_instance(): + data = {'reserver_name': 'changed name'} + result = has_reservation_data_changed(data, None) + assert result is False + + +@pytest.mark.django_db +def test_has_reservation_data_changed_empty_data(reservation_basic): + data = {} + result = has_reservation_data_changed(data, reservation_basic) + assert result is False