From 6b5f559aa7ac88e0ae925cf9878c739980bb1b76 Mon Sep 17 00:00:00 2001 From: SanttuA Date: Fri, 18 Aug 2023 11:18:02 +0300 Subject: [PATCH 01/13] Allowed unit managers to see all reservations (#274) Changes: - unit managers and admins can now see all reservations made to their resources, including denied and cancelled reservations --- resources/api/reservation.py | 120 ++++++++++--------- resources/tests/test_reservation_api.py | 148 ++++++++++++++++++------ 2 files changed, 177 insertions(+), 91 deletions(-) diff --git a/resources/api/reservation.py b/resources/api/reservation.py index aefd69754..8dd66b642 100644 --- a/resources/api/reservation.py +++ b/resources/api/reservation.py @@ -37,14 +37,14 @@ from resources.models import ( Reservation, Resource, ReservationMetadataSet, - ReservationHomeMunicipalityField, ReservationBulk + ReservationHomeMunicipalityField, ReservationBulk, Unit ) from resources.models.reservation import RESERVATION_BILLING_FIELDS, RESERVATION_EXTRA_FIELDS from resources.models.utils import build_reservations_ical_file from resources.pagination import ReservationPagination from resources.models.utils import generate_reservation_xlsx, get_object_or_none -from ..auth import is_general_admin, is_underage, is_overage, is_authenticated_user +from ..auth import is_general_admin, is_underage, is_overage, is_authenticated_user, is_any_admin, is_any_manager from .base import ( NullableDateTimeField, TranslatedModelSerializer, register_view, DRFFilterBooleanWidget, ExtraDataMixin @@ -97,6 +97,7 @@ class Meta: fields = [ 'bucket' ] + def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -107,6 +108,7 @@ def get_bucket(self, obj): (res.begin, res.end) ) + class HomeMunicipalitySerializer(TranslatedModelSerializer): class Meta: model = ReservationHomeMunicipalityField @@ -129,16 +131,17 @@ def to_internal_value(self, data): raise ValidationError(_('Invalid home municipality object - id is missing.')) if not home_municipality: - raise ValidationError({ - 'home_municipality': { - 'id': [_('Invalid pk "{pk_value}" - object does not exist.').format(pk_value=data)] - } - }) + raise ValidationError({ + 'home_municipality': { + 'id': [_('Invalid pk "{pk_value}" - object does not exist.').format(pk_value=data)] + } + }) data = home_municipality return data else: return super().to_internal_value(data) + class ReservationSerializer(ExtraDataMixin, TranslatedModelSerializer, munigeo_api.GeoModelSerializer): begin = NullableDateTimeField() end = NullableDateTimeField() @@ -186,7 +189,6 @@ def __init__(self, *args, **kwargs): self.handle_reservation_modify_request(request, resource) order = request.data.get('order') - begin, end = (request.data.get('begin', None), request.data.get('end', None)) if not order or isinstance(order, str) or (order and is_free(get_price(order, begin=begin, end=end))): required = [field for field in required if field not in RESERVATION_BILLING_FIELDS] @@ -207,15 +209,14 @@ def __init__(self, *args, **kwargs): self.context.update({'resource': resource}) - def handle_reservation_modify_request(self, request, resource): # handle removing order from data when updating reservation without paying if self.instance and resource.has_products() and 'order' in request.data: state = request.data.get('state') # states where reservation updates can be made if state in ( - Reservation.CONFIRMED, Reservation.CANCELLED, Reservation.DENIED, - Reservation.REQUESTED, Reservation.READY_FOR_PAYMENT, Reservation.WAITING_FOR_CASH_PAYMENT): + Reservation.CONFIRMED, Reservation.CANCELLED, Reservation.DENIED, + Reservation.REQUESTED, Reservation.READY_FOR_PAYMENT, Reservation.WAITING_FOR_CASH_PAYMENT): has_staff_perms = resource.is_manager(request.user) or resource.is_admin(request.user) user_can_modify = self.instance.can_modify(request.user) # staff members never pay after reservation creation and their order can be removed safely here @@ -223,11 +224,9 @@ def handle_reservation_modify_request(self, request, resource): if has_staff_perms or (user_can_modify and state != Reservation.READY_FOR_PAYMENT): del request.data['order'] - def get_required_fields(self): return [field_name for field_name in self.fields if self.fields[field_name].required] - def get_extra_fields(self, includes, context): from .resource import ResourceInlineSerializer @@ -339,7 +338,6 @@ def validate(self, data): if not region_code_for_country_code(phonenumbers.parse(reserver_phone_number).country_code): raise ValidationError(dict(reserver_phone_number=_('Invalid country code'))) - if data.get('staff_event', False): if not resource.can_create_staff_event(request_user): raise ValidationError(dict(staff_event=_('Only allowed to be set by resource managers'))) @@ -527,6 +525,7 @@ def get_user_permissions(self, obj): 'can_delete': can_modify_and_delete, } + class UserFilterBackend(filters.BaseFilterBackend): """ Filter by user uuid and by is_own. @@ -601,6 +600,7 @@ def filter_queryset(self, request, queryset, view): queryset = queryset.filter(begin__lte=times['end']) return queryset + class HasArrivedFilterBackend(filters.BaseFilterBackend): def filter_queryset(self, request, queryset, view): has_arrived = request.query_params.get('has_arrived', None) @@ -609,14 +609,17 @@ def filter_queryset(self, request, queryset, view): return queryset.filter(has_arrived=has_arrived) return queryset + class PhonenumberFilterBackend(filters.BaseFilterBackend): def filter_queryset(self, request, queryset, view): phonenumber = request.query_params.get('reserver_phone_number', '') phonenumber = phonenumber.strip() if phonenumber and phonenumber.isdigit(): - queryset = queryset.filter(Q(reserver_phone_number=phonenumber) | Q(reserver_phone_number='+%s' % phonenumber)) + queryset = queryset.filter(Q(reserver_phone_number=phonenumber) | + Q(reserver_phone_number='+%s' % phonenumber)) return queryset + class NeedManualConfirmationFilterBackend(filters.BaseFilterBackend): def filter_queryset(self, request, queryset, view): filter_value = request.query_params.get('need_manual_confirmation', None) @@ -730,13 +733,13 @@ def has_permission(self, request, view): resource = Resource.objects.get(pk=resource_id) except Resource.DoesNotExist: return request.method in permissions.SAFE_METHODS or \ - request.user and request.user.is_authenticated + request.user and request.user.is_authenticated if resource.authentication == 'strong' and \ - not request.user.is_strong_auth: + not request.user.is_strong_auth: return False if request.method in permissions.SAFE_METHODS or \ - request.user and request.user.is_authenticated: + request.user and request.user.is_authenticated: return True return request.method == 'POST' and resource.authentication == 'unauthenticated' @@ -801,6 +804,7 @@ def _get_cache_context(self): self._preload_permissions() return context + class ReservationBulkViewSet(viewsets.ModelViewSet, ReservationCacheMixin): queryset = ReservationBulk.objects.all() permission_classes = (permissions.IsAuthenticatedOrReadOnly, ReservationPermission, permissions.IsAdminUser,) @@ -815,8 +819,6 @@ def get_queryset(self): queryset = super().get_queryset() return queryset - - """ { "reservation_stack": [{ @@ -847,9 +849,9 @@ def create(self, request): stack[0].pop('resource') if len(stack) > 100: return JsonResponse({ - 'status':'false', - 'recurring_validation_error': _('Reservation failed. Too many reservations at once.'), - }, status=400 + 'status': 'false', + 'recurring_validation_error': _('Reservation failed. Too many reservations at once.'), + }, status=400 ) data = { **request.data @@ -864,9 +866,9 @@ def create(self, request): end = key.get('end') if begin is None or end is None: return JsonResponse({ - 'status':'false', - 'recurring_validation_error': _('Reservation failed. Begin or end time is missing.') - }, status=400 + 'status': 'false', + 'recurring_validation_error': _('Reservation failed. Begin or end time is missing.') + }, status=400 ) reservations = [] for key in stack: @@ -884,24 +886,24 @@ def create(self, request): res.end = end if resource.validate_reservation_period(res, res.user): return JsonResponse({ - 'status':'false', - 'recurring_validation_error': _('Reservation failed. Make sure reservation period is correct.') - }, status=400 + 'status': 'false', + 'recurring_validation_error': _('Reservation failed. Make sure reservation period is correct.') + }, status=400 ) if resource.validate_max_reservations_per_user(res.user): return JsonResponse({ - 'status':'false', - 'recurring_validation_error': _('Reservation failed. Too many reservations at once.') - }, status=400 + 'status': 'false', + 'recurring_validation_error': _('Reservation failed. Too many reservations at once.') + }, status=400 ) if resource.check_reservation_collision(begin, end, res): return JsonResponse({ - 'status':'false', - 'recurring_validation_error': _('Reservation failed. Overlap with existing reservations.') - }, status=400 + 'status': 'false', + 'recurring_validation_error': _('Reservation failed. Overlap with existing reservations.') + }, status=400 ) reservations.append(res) - reservation_dates_context = { 'dates': [] } + reservation_dates_context = {'dates': []} """ {% if bulk_email_context is defined %} @@ -915,27 +917,27 @@ def create(self, request): res.state = 'confirmed' if resource.validate_reservation_period(res, res.user): return JsonResponse({ - 'status':'false', - 'recurring_validation_error': _('Reservation failed. Make sure reservation period is correct.') - }, status=400 + 'status': 'false', + 'recurring_validation_error': _('Reservation failed. Make sure reservation period is correct.') + }, status=400 ) if resource.validate_max_reservations_per_user(res.user): return JsonResponse({ - 'status':'false', - 'recurring_validation_error': _('Reservation failed. Too many reservations at once.') - }, status=400 + 'status': 'false', + 'recurring_validation_error': _('Reservation failed. Too many reservations at once.') + }, status=400 ) if resource.check_reservation_collision(begin, end, res): return JsonResponse({ - 'status':'false', - 'recurring_validation_error': _('Reservation failed. Overlap with existing reservations.') - }, status=400 + 'status': 'false', + 'recurring_validation_error': _('Reservation failed. Overlap with existing reservations.') + }, status=400 ) res.save() reservation_dates_context['dates'].append( { 'begin': dateparser(reservations[0].begin, res.begin), - 'end': dateparser(reservations[0].end, res.end) + 'end': dateparser(reservations[0].end, res.end) } ) @@ -952,7 +954,8 @@ def create(self, request): } }) res = reservations[0] - url = ''.join([request.is_secure() and 'https' or 'http', get_current_site(request).domain, '/v1/', 'reservation/', str(res.id), '/']) + url = ''.join([request.is_secure() and 'https' or 'http', get_current_site( + request).domain, '/v1/', 'reservation/', str(res.id), '/']) ical_file = build_reservations_ical_file(reservations) attachment = ('reservation.ics', ical_file, 'text/calendar') res.send_reservation_mail( @@ -962,7 +965,8 @@ def create(self, request): extra_context=reservation_dates_context ) return JsonResponse( - data={ **ReservationSerializer(context={'request': self.request if self.request else request}).to_representation(res) }, + data={ + **ReservationSerializer(context={'request': self.request if self.request else request}).to_representation(res)}, status=200) except Exception as ex: return JsonResponse( @@ -1035,14 +1039,17 @@ def get_queryset(self): return queryset # normal users can see only their own reservations and reservations that are confirmed, requested or - # waiting for payment + # waiting for payment. Unit admins and managers can see all reservations of their units. filters = Q(state__in=(Reservation.CONFIRMED, Reservation.REQUESTED, - Reservation.WAITING_FOR_PAYMENT, Reservation.READY_FOR_PAYMENT, - Reservation.WAITING_FOR_CASH_PAYMENT)) + Reservation.WAITING_FOR_PAYMENT, Reservation.READY_FOR_PAYMENT, + Reservation.WAITING_FOR_CASH_PAYMENT)) if user.is_authenticated: filters |= Q(user=user) - queryset = queryset.filter(filters) + if is_any_admin(user) or is_any_manager(user): + filters |= Q(resource__unit__in=Unit.objects.managed_by(user)) + + queryset = queryset.filter(filters) queryset = queryset.filter(resource__in=Resource.objects.visible_for(user)) return queryset @@ -1066,13 +1073,13 @@ def perform_create(self, serializer): if order and order.state != Order.CONFIRMED and not resource.can_bypass_manual_confirmation(user): new_state = Reservation.WAITING_FOR_PAYMENT elif order and order.state == Order.WAITING and order.payment_method == Order.CASH \ - and resource.cash_payments_allowed and resource.can_bypass_manual_confirmation(user): + and resource.cash_payments_allowed and resource.can_bypass_manual_confirmation(user): new_state = Reservation.WAITING_FOR_CASH_PAYMENT else: new_state = Reservation.CONFIRMED if new_state == Reservation.CONFIRMED and \ - order and order.state == Order.WAITING and not resource.can_bypass_manual_confirmation(user): + order and order.state == Order.WAITING and not resource.can_bypass_manual_confirmation(user): new_state = Reservation.WAITING_FOR_PAYMENT instance.set_state(new_state, self.request.user) @@ -1086,7 +1093,7 @@ def perform_update(self, serializer): # when staff makes an update (can edit paid perm), state should not change to waiting for payment if new_state == Reservation.READY_FOR_PAYMENT and \ - order and order.state == Order.WAITING and not can_edit_paid: + order and order.state == Order.WAITING and not can_edit_paid: new_state = Reservation.WAITING_FOR_PAYMENT if new_state == Reservation.CONFIRMED and order and order.state == Order.WAITING: @@ -1100,7 +1107,7 @@ def perform_update(self, serializer): new_instance.set_state(new_state, self.request.user) if old_instance.state == Reservation.WAITING_FOR_CASH_PAYMENT and \ - new_state == Reservation.CONFIRMED: + new_state == Reservation.CONFIRMED: new_instance.get_order().set_state(Order.CONFIRMED, 'Cash payment confirmed.') # Reservation was modified, don't send modified upon patch. @@ -1132,5 +1139,6 @@ def send_modified_mail(self, new_instance, is_staff=False): if not is_staff: new_instance.notify_staff_about_reservation(NotificationType.RESERVATION_MODIFIED_OFFICIAL) + register_view(ReservationViewSet, 'reservation') register_view(ReservationBulkViewSet, 'reservation_bulk') diff --git a/resources/tests/test_reservation_api.py b/resources/tests/test_reservation_api.py index 733b9fa71..3ef16c687 100644 --- a/resources/tests/test_reservation_api.py +++ b/resources/tests/test_reservation_api.py @@ -191,6 +191,24 @@ def reservations_in_all_states(resource_in_unit, user): return reservations +@pytest.fixture +def reservations_in_all_states2(resource_in_unit2, user): + all_states = ( + Reservation.CANCELLED, Reservation.CONFIRMED, Reservation.DENIED, Reservation.REQUESTED, + Reservation.WAITING_FOR_PAYMENT, Reservation.WAITING_FOR_CASH_PAYMENT + ) + reservations = dict() + for i, state in enumerate(all_states, 1): + reservations[state] = Reservation.objects.create( + resource=resource_in_unit2, + begin='2115-04-0%sT09:00:00+02:00' % i, + end='2115-04-0%sT10:00:00+02:00' % i, + user=user, + state=state + ) + return reservations + + @pytest.fixture def reservation_created_notification(): with translation.override('en'): @@ -466,6 +484,7 @@ def test_comments_are_only_for_admins( response = api_client.get(response.data['url']) assert 'comments' not in response.data + @pytest.mark.parametrize('user_fixture, expected_status', [ (None, 401), ('user', 400), @@ -473,7 +492,6 @@ def test_comments_are_only_for_admins( ('staff_user', 201), ('unit4_manager_user', 201), ]) - @pytest.mark.django_db def test_comments_can_be_created_by_correct_people_when_resource_sets_is_reservable_by_all( api_client, list_url, resource_in_unit, resource_in_unit3, reservation_data, user_fixture, expected_status, @@ -494,7 +512,6 @@ def test_comments_can_be_created_by_correct_people_when_resource_sets_is_reserva resource_in_unit.reservable_by_all_staff = True resource_in_unit.save() - test_comment = 'test comment abc' reservation_data.update({ 'comments': test_comment, @@ -520,6 +537,7 @@ def test_comments_can_be_created_by_correct_people_when_resource_sets_is_reserva if response.status_code == 201: assert response.data['comments'] == test_comment + @pytest.mark.parametrize('user_fixture, expected_status_post, expected_status_put ', [ (None, 401, 400), ('user', 400, 400), @@ -527,7 +545,6 @@ def test_comments_can_be_created_by_correct_people_when_resource_sets_is_reserva ('staff_user', 201, 200), ('unit4_manager_user', 201, 200), ]) - @pytest.mark.django_db def test_comments_can_be_updated_by_correct_people_when_resource_sets_is_reservable_by_all( api_client, list_url, resource_in_unit, resource_in_unit3, reservation_data, user_fixture, expected_status_post, expected_status_put, @@ -580,6 +597,7 @@ def test_comments_can_be_updated_by_correct_people_when_resource_sets_is_reserva if response.status_code == 200: assert response.data['comments'] == updated_comment + @pytest.mark.django_db def test_anon_and_other_users_cannot_see_virtual_event_data(api_client, reservation, user2): """ @@ -918,7 +936,6 @@ def test_strong_reservation(api_client, list_url, reservation_data, strong_user, assert response.status_code == 403 - @pytest.mark.django_db def test_reservation_user_filter(api_client, list_url, reservation, resource_in_unit, user, user2): """ @@ -1164,7 +1181,8 @@ def test_staff_event_restrictions(user_api_client, staff_api_client, staff_user, assert set(DEFAULT_REQUIRED_RESERVATION_EXTRA_FIELDS) == set(response.data) # unit manager but reserver_name and event_description missing - UnitAuthorization.objects.create(subject=resource_in_unit.unit, level=UnitAuthorizationLevel.manager, authorized=staff_user) + UnitAuthorization.objects.create(subject=resource_in_unit.unit, + level=UnitAuthorizationLevel.manager, authorized=staff_user) response = staff_api_client.post(list_url, data=reservation_data) assert response.status_code == 400 assert {'reserver_name', 'event_description'} == set(response.data) @@ -1185,7 +1203,8 @@ def test_new_staff_event_gets_confirmed(user_api_client, staff_api_client, staff reservation.delete() - UnitAuthorization.objects.create(subject=resource_in_unit.unit, level=UnitAuthorizationLevel.manager, authorized=staff_user) + UnitAuthorization.objects.create(subject=resource_in_unit.unit, + level=UnitAuthorizationLevel.manager, authorized=staff_user) reservation_data['staff_event'] = True reservation_data['reserver_name'] = 'herra huu' reservation_data['event_description'] = 'herra huun bileet' @@ -1255,6 +1274,64 @@ def test_admins_can_see_reservations_in_all_states( assert response.data['count'] == 6 +@pytest.mark.django_db +def test_unit_manager_can_see_all_reservations_in_their_unit( + resource_in_unit, api_client, staff_user, reservations_in_all_states, list_url): + """ + Tests that unit managers can see reservations made to their resources in all 6 reservation states + """ + UnitAuthorization.objects.create(subject=resource_in_unit.unit, + level=UnitAuthorizationLevel.manager, authorized=staff_user) + api_client.force_authenticate(user=staff_user) + response = api_client.get(list_url) + assert response.status_code == 200 + assert response.data['count'] == 6 + + +@pytest.mark.django_db +def test_unit_admin_can_see_all_reservations_in_their_unit( + resource_in_unit, api_client, staff_user, reservations_in_all_states, list_url): + """ + Tests that unit admins can see reservations made to their resources in all 6 reservation states + """ + UnitAuthorization.objects.create(subject=resource_in_unit.unit, + level=UnitAuthorizationLevel.admin, authorized=staff_user) + api_client.force_authenticate(user=staff_user) + response = api_client.get(list_url) + assert response.status_code == 200 + assert response.data['count'] == 6 + + +@pytest.mark.django_db +def test_unit_manager_cannot_see_all_reservations_not_in_their_unit( + resource_in_unit, api_client, staff_user, reservations_in_all_states2, list_url): + """ + Tests that unit managers can only see reservations made to other than their own unit in the + 4 allowed reservation states + """ + UnitAuthorization.objects.create(subject=resource_in_unit.unit, + level=UnitAuthorizationLevel.manager, authorized=staff_user) + api_client.force_authenticate(user=staff_user) + response = api_client.get(list_url) + assert response.status_code == 200 + assert response.data['count'] == 4 + + +@pytest.mark.django_db +def test_unit_admin_cannot_see_all_reservations_not_in_their_unit( + resource_in_unit, api_client, staff_user, reservations_in_all_states2, list_url): + """ + Tests that unit admins can only see reservations made to other than their own unit in the + 4 allowed reservation states + """ + UnitAuthorization.objects.create(subject=resource_in_unit.unit, + level=UnitAuthorizationLevel.admin, authorized=staff_user) + api_client.force_authenticate(user=staff_user) + response = api_client.get(list_url) + assert response.status_code == 200 + assert response.data['count'] == 4 + + @pytest.mark.django_db def test_reservation_cannot_be_confirmed_without_permission( api_client, user_api_client, detail_url, reservation, @@ -1296,7 +1373,7 @@ def test_reservation_patch_has_arrived(api_client, general_admin, detail_url, re reservation.save() api_client.force_authenticate(user=general_admin) response = api_client.patch(detail_url, data={ - 'has_arrived':True + 'has_arrived': True }) assert response.status_code == 200 reservation.refresh_from_db() @@ -1309,7 +1386,7 @@ def test_reservation_patch_fail_has_arrived(api_client, user2, detail_url, reser reservation.save() api_client.force_authenticate(user=user2) response = api_client.patch(detail_url, data={ - 'has_arrived':True + 'has_arrived': True }) assert response.status_code == 403 reservation.refresh_from_db() @@ -1776,10 +1853,11 @@ def test_reservation_mail_images(user_api_client, user, list_url, reservation_da @override_settings(RESPA_MAILS_ENABLED=True) @pytest.mark.django_db def test_reservation_modified_email_by_official(reservation, reservation_data, staff_api_client, staff_user, - reservation_modified_by_official_notification): + reservation_modified_by_official_notification): reservation.reserver_email_address = 'test@tester.com' reservation.save() - UnitAuthorization.objects.create(subject=reservation.resource.unit, level=UnitAuthorizationLevel.manager, authorized=staff_user) + UnitAuthorization.objects.create(subject=reservation.resource.unit, + level=UnitAuthorizationLevel.manager, authorized=staff_user) reservation_data['preferred_language'] = 'en' reservation_data['reserver_name'] = 'new name' detail_url = reverse('reservation-detail', kwargs={'pk': reservation.pk}) @@ -1796,10 +1874,11 @@ def test_reservation_modified_email_by_official(reservation, reservation_data, s @override_settings(RESPA_MAILS_ENABLED=True) @pytest.mark.django_db def test_reservation_modified_email_by_official_comment_only(reservation, reservation_data, staff_api_client, staff_user, - reservation_modified_by_official_notification): + reservation_modified_by_official_notification): reservation.reserver_email_address = 'test@tester.com' reservation.save() - UnitAuthorization.objects.create(subject=reservation.resource.unit, level=UnitAuthorizationLevel.manager, authorized=staff_user) + UnitAuthorization.objects.create(subject=reservation.resource.unit, + level=UnitAuthorizationLevel.manager, authorized=staff_user) reservation_data['preferred_language'] = 'en' reservation_data['comments'] = 'test comment' reservation_data['begin'] = reservation.begin @@ -1937,7 +2016,6 @@ def test_access_code_visibility( def test_reservation_created_with_access_code_mail(user_api_client, user, resource_in_unit, list_url, reservation_data): reservation_data['preferred_language'] = 'en' - # The mail should not be sent if access code type is none response = user_api_client.post(list_url, data=reservation_data) assert response.status_code == 201 @@ -2351,7 +2429,8 @@ def test_manager_can_make_staff_reservation( reservation_data['staff_event'] = True reservation_data['reserver_name'] = 'herra huu' reservation_data['event_description'] = 'herra huun bileet' - UnitAuthorization.objects.create(subject=resource_in_unit.unit, level=UnitAuthorizationLevel.manager, authorized=staff_user) + UnitAuthorization.objects.create(subject=resource_in_unit.unit, + level=UnitAuthorizationLevel.manager, authorized=staff_user) response = staff_api_client.post(list_url, data=reservation_data) assert response.status_code == 201, "Request failed with: %s" % (str(response.content, 'utf8')) assert response.data.get('staff_event', False) is True @@ -2375,8 +2454,6 @@ def test_resource_filter(resource_in_unit, resource_in_unit2, other_resource, re assert_response_objects(response, (reservation, reservation2)) - - @pytest.mark.django_db def test_reservation_default_type(reservation_data, user_api_client): """ Reservation should return default reservation type """ @@ -2588,6 +2665,7 @@ def test_viewer_can_view_reservation_user(api_client, reservation, unit_viewer_u assert response.status_code == 200 assert response.data['user']['email'] == user.email + @pytest.mark.django_db def test_viewer_can_modify_reservations(resource_in_unit, resource_in_unit2, api_client, unit_viewer_user, reservation_data, detail_url): # unit_viewer_user is viewer of resource_in_unit (resource of reservation_data) @@ -2976,8 +3054,8 @@ def test_disallow_overlapping_reservations(resource_in_unit, resource_in_unit2, @pytest.mark.django_db def test_reservation_can_be_made_to_unit_with_overlap_restriction_when_no_overlap( - api_client, reservation_data, user, list_url, resource_in_unit4_1 - ): + api_client, reservation_data, user, list_url, resource_in_unit4_1 +): """ Tests that a reservation can be made to a resource in unit with overlap restriction when there is no overlap @@ -2998,9 +3076,9 @@ def test_reservation_can_be_made_to_unit_with_overlap_restriction_when_no_overla ]) @pytest.mark.django_db def test_reservations_made_to_unit_with_overlap_restriction_when_overlapping( - resource, is_staff, expected, api_client, reservation_data, - user, unit4_manager_user, list_url, resource_in_unit4_1, resource_in_unit4_2, reservation4 - ): + resource, is_staff, expected, api_client, reservation_data, + user, unit4_manager_user, list_url, resource_in_unit4_1, resource_in_unit4_2, reservation4 +): """ Tests that reservations are handled correctly to resources in units with overlap restrictions @@ -3019,8 +3097,8 @@ def test_reservations_made_to_unit_with_overlap_restriction_when_overlapping( @pytest.mark.django_db def test_reservation_can_be_updated_in_unit_with_overlap_restriction_when_no_overlap( - api_client, reservation_data, user, resource_in_unit4_1, reservation4 - ): + api_client, reservation_data, user, resource_in_unit4_1, reservation4 +): """ Tests that a reservation can be updated to a resource in unit with overlap restriction when there is no overlap @@ -3040,8 +3118,8 @@ def test_reservation_can_be_updated_in_unit_with_overlap_restriction_when_no_ove @pytest.mark.django_db def test_reservation_can_be_made_to_unit_with_per_user_overlap_restriction_when_no_overlap( - api_client, reservation_data, user, list_url, resource_in_unit4_1 - ): + api_client, reservation_data, user, list_url, resource_in_unit4_1 +): """ Tests that a reservation can be made to a resource in unit with per user overlap restriction when there is no overlap @@ -3062,9 +3140,9 @@ def test_reservation_can_be_made_to_unit_with_per_user_overlap_restriction_when_ ]) @pytest.mark.django_db def test_reservations_made_to_unit_with_per_user_overlap_restriction( - api_client, reservation_data, user, user2, list_url, test_unit4, resource_in_unit4_2, - reservation4, same_user, expected - ): + api_client, reservation_data, user, user2, list_url, test_unit4, resource_in_unit4_2, + reservation4, same_user, expected +): """ Tests that reservations are handled correctly to a resource in unit with per user overlap restriction when same or other user has made a reservation to another resource in the same unit with overlapping time @@ -3104,8 +3182,8 @@ def test_reservation_can_be_made_to_unit_with_per_user_overlap_restriction_and_a @pytest.mark.django_db def test_reservation_can_be_updated_in_unit_with_per_user_overlap_restriction_when_no_overlap( - api_client, reservation_data, user, resource_in_unit4_1, reservation4, test_unit4 - ): + api_client, reservation_data, user, resource_in_unit4_1, reservation4, test_unit4 +): """ Tests that a reservation can be updated to a resource in unit with per user overlap restriction when there is no overlap @@ -3127,8 +3205,8 @@ def test_reservation_can_be_updated_in_unit_with_per_user_overlap_restriction_wh @pytest.mark.django_db def test_reservation_cannot_be_updated_in_unit_with_per_user_overlap_restriction_when_overlapping( - api_client, reservation_data, user, resource_in_unit4_1, reservation4, reservation5, test_unit4 - ): + api_client, reservation_data, user, resource_in_unit4_1, reservation4, reservation5, test_unit4 +): """ Tests that a reservation cannot be updated to a resource in unit with per user overlap restriction when there is overlap @@ -3165,9 +3243,9 @@ def test_reservation_cannot_be_updated_in_unit_with_per_user_overlap_restriction ]) @pytest.mark.django_db def test_reservations_made_to_unit_with_overlap_restriction_with_different_times( - begin, end, expected, api_client, reservation_data, user, list_url, - resource_in_unit4_1, resource_in_unit4_2, reservation4, reservation5 - ): + begin, end, expected, api_client, reservation_data, user, list_url, + resource_in_unit4_1, resource_in_unit4_2, reservation4, reservation5 +): """ Tests that reservations are handled correctly to resources in units with overlap restrictions when reserving different length reservations and with different overlaps From 9a0c119b3d4821b2a7456ec1e72d5f25a5c72e08 Mon Sep 17 00:00:00 2001 From: SanttuA Date: Wed, 23 Aug 2023 11:26:31 +0300 Subject: [PATCH 02/13] Changed admin metadata set select widget (#276) Changes: - metadata set supported and required fields use horizontal FilteredSelectMultiple widget instead of the default multiple select elements --- resources/admin/__init__.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/resources/admin/__init__.py b/resources/admin/__init__.py index 8e51fd816..26fb12f1d 100644 --- a/resources/admin/__init__.py +++ b/resources/admin/__init__.py @@ -5,6 +5,7 @@ from django.contrib import admin from django.contrib.admin import site as admin_site from django.contrib.admin.utils import unquote +from django.contrib.admin.widgets import FilteredSelectMultiple from django.contrib.auth import get_user_model from django.contrib.auth.models import Group from django.contrib.gis.admin import OSMGeoAdmin @@ -229,7 +230,7 @@ def _get_inlines(self, obj): def has_change_permission(self, request, obj=None): return super().has_change_permission(request, obj) and (obj and not obj.soft_deleted) - + def has_delete_permission(self, request, obj=None): return False @@ -378,6 +379,11 @@ class TermsOfUseAdmin(PopulateCreatedAndModifiedMixin, CommonExcludeMixin, Trans class ReservationMetadataSetForm(forms.ModelForm): + supported_fields = forms.ModelMultipleChoiceField( + ReservationMetadataField.objects.all(), widget=FilteredSelectMultiple(_('Supported fields'), False)) + required_fields = forms.ModelMultipleChoiceField( + ReservationMetadataField.objects.all(), widget=FilteredSelectMultiple(_('Required fields'), False)) + class Meta: model = ReservationMetadataSet exclude = CommonExcludeMixin.exclude + ('id',) @@ -576,4 +582,4 @@ class MaintenanceMessageAdmin(TranslationAdmin): admin_site.register(Token, RespaTokenAdmin) admin_site.register(MaintenanceMessage, MaintenanceMessageAdmin) admin_site.register(UniversalFormFieldType, UniversalFieldAdmin) -admin_site.register(ResourceUniversalFormOption, ResourceUniversalFormOptionAdmin) \ No newline at end of file +admin_site.register(ResourceUniversalFormOption, ResourceUniversalFormOptionAdmin) From 22d452602242a26547eb290099ad909858f228f2 Mon Sep 17 00:00:00 2001 From: ezkat <50319957+ezkat@users.noreply.github.com> Date: Wed, 23 Aug 2023 11:35:47 +0300 Subject: [PATCH 03/13] Change resources admin widget (#277) --- payments/admin.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/payments/admin.py b/payments/admin.py index 5250278d3..efbba4816 100644 --- a/payments/admin.py +++ b/payments/admin.py @@ -125,7 +125,10 @@ class Meta: def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.fields['resources'] = forms.ModelMultipleChoiceField(queryset=Resource.objects.order_by('name')) + self.fields['resources'] = forms.ModelMultipleChoiceField( + queryset=Resource.objects.order_by('name'), + widget=admin.widgets.FilteredSelectMultiple(_('resources'), False) + ) def clean(self): super().clean() @@ -176,6 +179,7 @@ class ProductAdmin(TranslationAdmin): 'max_quantity', 'get_resources', 'get_created_at', 'get_modified_at', 'price_tax_free' ) readonly_fields = ('product_id',) + filter_horizontal = ('resources',) fieldsets = ( (None, { 'fields': ('sku', 'type', 'name', 'description', 'max_quantity') From 4ea48e9f5d47e87b6672ac2094d6500938d9399f Mon Sep 17 00:00:00 2001 From: SanttuA Date: Wed, 23 Aug 2023 12:27:49 +0300 Subject: [PATCH 04/13] Fixed per period product quantity pricing issue (#275) Changes: - quantity is now added when quantity is over 1 to detailed pricing for per period products without timeslot pricing - orderline rounded price api returns price*quantity for per period products instead of always returning pricing of one product --- payments/api/base.py | 4 ++ payments/models.py | 3 + payments/tests/test_order_api.py | 97 ++++++++++++++++++++++++++++++++ payments/tests/test_product.py | 32 +++++++++++ 4 files changed, 136 insertions(+) diff --git a/payments/api/base.py b/payments/api/base.py index ec334ede9..b836f4eeb 100644 --- a/payments/api/base.py +++ b/payments/api/base.py @@ -73,6 +73,10 @@ def get_rounded_price(self, obj) -> str: for x in detailed_prices: temp = detailed_prices[x]['tax_total'] + detailed_prices[x]['taxfree_price_total'] price += temp + + # quantity needs to be handled separately for per period products + if obj.product.price_type != Product.PRICE_FIXED: + price *= obj.quantity return '{:.2f}'.format(price) def to_representation(self, instance): diff --git a/payments/models.py b/payments/models.py index 067ff66c3..c22738a2c 100644 --- a/payments/models.py +++ b/payments/models.py @@ -609,6 +609,9 @@ def get_detailed_price_for_time_range(self, begin: datetime, end: datetime, prod pretax=self.get_pretax_price_context(price, rounded=False), taxfree_price=self.price_tax_free ) + if quantity > 1: + # quantity is only defined/>1 if there are multiples of the same product + detailed_pricing['default']['quantity'] = quantity slot_begin += check_interval detailed_pricing = finalize_price_data(detailed_pricing, self.price_type, self.price_period) diff --git a/payments/tests/test_order_api.py b/payments/tests/test_order_api.py index 67f5cc386..a463b9dce 100644 --- a/payments/tests/test_order_api.py +++ b/payments/tests/test_order_api.py @@ -236,3 +236,100 @@ def test_order_price_check_with_fixed_price_product_returns_correct_price(begin, assert response.status_code == 200 assert len(response.data['order_lines']) == 2 assert response.data['price'] == price_result + + +@pytest.mark.parametrize('begin, end, customer_group, price', ( + (datetime(2022, 3, 1, 10, 0), datetime(2022, 3, 1, 12, 0), None, 20.00), + (datetime(2022, 3, 1, 10, 0), datetime(2022, 3, 1, 12, 0), 'cg-adults-1', 16.00), + (datetime(2022, 3, 1, 10, 0), datetime(2022, 3, 1, 12, 0), 'cg-children-1', 22.00), + (datetime(2022, 3, 1, 10, 0), datetime(2022, 3, 1, 12, 0), 'cg-elders-1', 12.00), + (datetime(2022, 3, 1, 10, 0), datetime(2022, 3, 1, 12, 0), 'cg-companies-1', 20.00), + (datetime(2022, 3, 1, 14, 0), datetime(2022, 3, 1, 16, 0), None, 30.00), + (datetime(2022, 3, 1, 14, 0), datetime(2022, 3, 1, 16, 0), 'cg-adults-1', 24.00), + (datetime(2022, 3, 1, 14, 0), datetime(2022, 3, 1, 16, 0), 'cg-children-1', 22.00), + (datetime(2022, 3, 1, 14, 0), datetime(2022, 3, 1, 16, 0), 'cg-elders-1', 30.00), + (datetime(2022, 3, 1, 14, 0), datetime(2022, 3, 1, 16, 0), 'cg-companies-1', 30.00), + (datetime(2022, 3, 1, 11, 30), datetime(2022, 3, 1, 12, 30), None, 12.50), + (datetime(2022, 3, 1, 11, 30), datetime(2022, 3, 1, 12, 30), 'cg-adults-1', 10.00), + (datetime(2022, 3, 1, 11, 30), datetime(2022, 3, 1, 12, 30), 'cg-children-1', 11.00), + (datetime(2022, 3, 1, 11, 30), datetime(2022, 3, 1, 12, 30), 'cg-elders-1', 10.50), + (datetime(2022, 3, 1, 11, 30), datetime(2022, 3, 1, 12, 30), 'cg-companies-1', 12.50) +)) +def test_order_price_check_per_period_price_product_rounded_price_is_correct(begin, end, customer_group, price, + product_with_pcgs_and_time_slot_prices, user_api_client, product_with_all_named_customer_groups): + ''' + Test the check price endpoint returns rough correct orderline rounded price for a product containing + a time slot and product customer groups. + ''' + for quantity in range(3): + price_check_data = { + "order_lines": [ + { + "product": product_with_pcgs_and_time_slot_prices.product_id, + "quantity": quantity + }, + { + "product": product_with_all_named_customer_groups.product_id, + "quantity": 0 + }, + ], + "begin": str(begin), + "end": str(end), + } + + if customer_group: + price_check_data['customer_group'] = customer_group + response = user_api_client.post(CHECK_PRICE_URL, price_check_data) + assert response.status_code == 200 + orderlines = response.data['order_lines'] + assert len(orderlines) == 2 + assert round(float(orderlines[0]['rounded_price'])) == round(price*quantity) + + +@pytest.mark.parametrize('begin, end, customer_group, price', ( + (datetime(2022, 3, 1, 7, 0), datetime(2022, 3, 1, 8, 0), None, 50.25), + (datetime(2022, 3, 1, 7, 0), datetime(2022, 3, 1, 11, 0), None, 50.25), + (datetime(2022, 3, 1, 10, 0), datetime(2022, 3, 1, 11, 0), None, 10.00), + (datetime(2022, 3, 1, 10, 0), datetime(2022, 3, 1, 12, 0), None, 10.00), + (datetime(2022, 3, 1, 12, 0), datetime(2022, 3, 1, 13, 0), None, 12.00), + (datetime(2022, 3, 1, 12, 0), datetime(2022, 3, 1, 15, 30), None, 11.50), + (datetime(2022, 3, 1, 14, 0), datetime(2022, 3, 1, 16, 0), None, 14.00), + (datetime(2022, 3, 1, 10, 0), datetime(2022, 3, 1, 16, 0), 'cg-adults-1', 50.25), + (datetime(2022, 3, 1, 14, 0), datetime(2022, 3, 1, 15, 0), 'cg-adults-1', 8.00), + (datetime(2022, 3, 1, 14, 0), datetime(2022, 3, 1, 16, 0), 'cg-adults-1', 8.00), + (datetime(2022, 3, 1, 15, 0), datetime(2022, 3, 1, 16, 0), 'cg-adults-1', 7.00), + (datetime(2022, 3, 1, 7, 0), datetime(2022, 3, 1, 8, 0), 'cg-children-1', 6.50), + (datetime(2022, 3, 1, 14, 0), datetime(2022, 3, 1, 16, 0), 'cg-children-1', 6.50), + (datetime(2022, 3, 1, 7, 0), datetime(2022, 3, 1, 8, 0), 'cg-elders-1', 50.25), + (datetime(2022, 3, 1, 10, 0), datetime(2022, 3, 1, 11, 0), 'cg-elders-1', 10.00), + (datetime(2022, 3, 1, 14, 0), datetime(2022, 3, 1, 16, 0), 'cg-elders-1', 14.00), +)) +def test_order_price_check_fixed_price_product_rounded_price_is_correct(begin, end, customer_group, price, + product_with_fixed_price_type_and_time_slots, user_api_client, product_with_all_named_customer_groups): + ''' + Test the check price endpoint returns rough correct orderline rounded price for a product containing + time slots, product customer groups and fixed pricing. + ''' + for quantity in range(3): + price_check_data = { + "order_lines": [ + { + "product": product_with_fixed_price_type_and_time_slots.product_id, + "quantity": quantity + }, + { + "product": product_with_all_named_customer_groups.product_id, + "quantity": 0 + }, + ], + "begin": str(begin), + "end": str(end), + } + + if customer_group: + price_check_data['customer_group'] = customer_group + response = user_api_client.post(CHECK_PRICE_URL, price_check_data) + assert response.status_code == 200 + orderlines = response.data['order_lines'] + assert len(orderlines) == 2 + assert round(float(orderlines[0]['rounded_price'])) == round(price*quantity) diff --git a/payments/tests/test_product.py b/payments/tests/test_product.py index bec7a171d..20f71a651 100644 --- a/payments/tests/test_product.py +++ b/payments/tests/test_product.py @@ -127,3 +127,35 @@ def test_get_pretax_price_for_reservation_success(product_1, two_hour_reservatio not_rounded = product_1.get_pretax_price_for_reservation(two_hour_reservation, rounded=False) assert rounded == Decimal('20.66') assert not_rounded.quantize(Decimal('0.00001')) == Decimal('20.66129') + + +def test_get_detailed_price_for_time_range_per_period_timeslots_quantity(product_with_pcgs_and_time_slot_prices): + """Test quantity is added correctly to detailed pricing with per period products with timeslots""" + begin = datetime.datetime(2119, 5, 5, 10, 0, 0, tzinfo=UTC) + end = datetime.datetime(2119, 5, 5, 12, 0, 0, tzinfo=UTC) + + result = product_with_pcgs_and_time_slot_prices.get_detailed_price_for_time_range(begin, end, quantity=0) + assert 'quantity' not in result['default'] + + result = product_with_pcgs_and_time_slot_prices.get_detailed_price_for_time_range(begin, end, quantity=1) + assert 'quantity' not in result['default'] + + result = product_with_pcgs_and_time_slot_prices.get_detailed_price_for_time_range(begin, end, quantity=2) + assert 'quantity' in result['default'] + assert result['default']['quantity'] == 2 + + +def test_get_detailed_price_for_time_range_per_period_no_timeslots_quantity(product_with_no_price_product_cg): + """Test quantity is added correctly to detailed pricing with per period products without timeslots""" + begin = datetime.datetime(2119, 5, 5, 10, 0, 0, tzinfo=UTC) + end = datetime.datetime(2119, 5, 5, 12, 0, 0, tzinfo=UTC) + + result = product_with_no_price_product_cg.get_detailed_price_for_time_range(begin, end, quantity=0) + assert 'quantity' not in result['default'] + + result = product_with_no_price_product_cg.get_detailed_price_for_time_range(begin, end, quantity=1) + assert 'quantity' not in result['default'] + + result = product_with_no_price_product_cg.get_detailed_price_for_time_range(begin, end, quantity=2) + assert 'quantity' in result['default'] + assert result['default']['quantity'] == 2 From e030a41d179faf04ec5b91ba0fc08a53c297cedf Mon Sep 17 00:00:00 2001 From: SanttuA Date: Wed, 23 Aug 2023 14:04:40 +0300 Subject: [PATCH 05/13] Fixed admin reservation metadata empty set issue (#278) Changes: - set supported and required fields to not be required - fixed empty field sets causing an error --- resources/admin/__init__.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/resources/admin/__init__.py b/resources/admin/__init__.py index 26fb12f1d..4790b52ad 100644 --- a/resources/admin/__init__.py +++ b/resources/admin/__init__.py @@ -380,17 +380,21 @@ class TermsOfUseAdmin(PopulateCreatedAndModifiedMixin, CommonExcludeMixin, Trans class ReservationMetadataSetForm(forms.ModelForm): supported_fields = forms.ModelMultipleChoiceField( - ReservationMetadataField.objects.all(), widget=FilteredSelectMultiple(_('Supported fields'), False)) + ReservationMetadataField.objects.all(), + widget=FilteredSelectMultiple(_('Supported fields'), False), + required=False) required_fields = forms.ModelMultipleChoiceField( - ReservationMetadataField.objects.all(), widget=FilteredSelectMultiple(_('Required fields'), False)) + ReservationMetadataField.objects.all(), + widget=FilteredSelectMultiple(_('Required fields'), False), + required=False) class Meta: model = ReservationMetadataSet exclude = CommonExcludeMixin.exclude + ('id',) def clean(self): - supported = set(self.cleaned_data.get('supported_fields')) - required = set(self.cleaned_data.get('required_fields')) + supported = set(self.cleaned_data.get('supported_fields', [])) + required = set(self.cleaned_data.get('required_fields', [])) if not required.issubset(supported): raise ValidationError(_('Required fields must be a subset of supported fields')) return self.cleaned_data From 6c6969a3ebb494d331dac695535875af39967e54 Mon Sep 17 00:00:00 2001 From: SanttuA Date: Thu, 31 Aug 2023 12:13:01 +0300 Subject: [PATCH 06/13] Changed reservation reserver id meaning (#279) Changes: - reserver id now refer to only business id instead of both business and person ssn --- locale/en/LC_MESSAGES/django.po | 2 +- locale/fi/LC_MESSAGES/django.po | 4 ++-- locale/sv/LC_MESSAGES/django.po | 4 ++-- .../0147_reserver_id_only_business.py | 18 ++++++++++++++++++ resources/models/reservation.py | 2 +- 5 files changed, 24 insertions(+), 6 deletions(-) create mode 100644 resources/migrations/0147_reserver_id_only_business.py diff --git a/locale/en/LC_MESSAGES/django.po b/locale/en/LC_MESSAGES/django.po index f148c4042..3d6a23936 100644 --- a/locale/en/LC_MESSAGES/django.po +++ b/locale/en/LC_MESSAGES/django.po @@ -1130,7 +1130,7 @@ msgid "Reserver name" msgstr "" #: resources/models/reservation.py:156 -msgid "Reserver ID (business or person)" +msgid "Reserver ID (business)" msgstr "" #: resources/models/reservation.py:157 diff --git a/locale/fi/LC_MESSAGES/django.po b/locale/fi/LC_MESSAGES/django.po index 07297f1e7..022617261 100644 --- a/locale/fi/LC_MESSAGES/django.po +++ b/locale/fi/LC_MESSAGES/django.po @@ -810,8 +810,8 @@ msgstr "Varauksen lisäkysymykset" msgid "Reserver name" msgstr "Varaajan nimi" -msgid "Reserver ID (business or person)" -msgstr "Varaajan tunniste (yritys tai henkilö)" +msgid "Reserver ID (business)" +msgstr "Varaajan tunniste (yritys)" msgid "Reserver email address" msgstr "Varaajan sähköpostiosoite" diff --git a/locale/sv/LC_MESSAGES/django.po b/locale/sv/LC_MESSAGES/django.po index 6091cfdaa..f6549637c 100644 --- a/locale/sv/LC_MESSAGES/django.po +++ b/locale/sv/LC_MESSAGES/django.po @@ -812,8 +812,8 @@ msgstr "Extra frågor för bokning" msgid "Reserver name" msgstr "Reservnamn" -msgid "Reserver ID (business or person)" -msgstr "Reserv ID (företag eller person)" +msgid "Reserver ID (business)" +msgstr "Reserv ID (företag)" msgid "Reserver email address" msgstr "Reserverares email adress" diff --git a/resources/migrations/0147_reserver_id_only_business.py b/resources/migrations/0147_reserver_id_only_business.py new file mode 100644 index 000000000..23a768ca8 --- /dev/null +++ b/resources/migrations/0147_reserver_id_only_business.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.20 on 2023-08-28 11:05 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('resources', '0146_add_private_event_field'), + ] + + operations = [ + migrations.AlterField( + model_name='reservation', + name='reserver_id', + field=models.CharField(blank=True, max_length=30, verbose_name='Reserver ID (business)'), + ), + ] diff --git a/resources/models/reservation.py b/resources/models/reservation.py index b1973b669..85c121517 100644 --- a/resources/models/reservation.py +++ b/resources/models/reservation.py @@ -170,7 +170,7 @@ class Reservation(ModifiableModel): # extra detail fields for manually confirmed reservations reserver_name = models.CharField(verbose_name=_('Reserver name'), max_length=100, blank=True) - reserver_id = models.CharField(verbose_name=_('Reserver ID (business or person)'), max_length=30, blank=True) + reserver_id = models.CharField(verbose_name=_('Reserver ID (business)'), max_length=30, blank=True) reserver_email_address = models.EmailField(verbose_name=_('Reserver email address'), blank=True) reserver_phone_number = models.CharField(verbose_name=_('Reserver phone number'), max_length=30, blank=True) reserver_address_street = models.CharField(verbose_name=_('Reserver address street'), max_length=100, blank=True) From a59509fa0dbf92c6dbd260c1ae08a604b6724996 Mon Sep 17 00:00:00 2001 From: ezkat <50319957+ezkat@users.noreply.github.com> Date: Tue, 5 Sep 2023 12:21:29 +0300 Subject: [PATCH 07/13] Feature: Add maintenance mode (#281) * Add maintenance mode * Create maintenance app * Add user perm test during maintenance mode --- maintenance/__init__.py | 0 maintenance/admin.py | 55 ++++++++++++++++ maintenance/api/__init__.py | 1 + .../api/announcements.py | 4 +- maintenance/apps.py | 6 ++ .../migrations/0001_create_maintenance_app.py | 57 +++++++++++++++++ maintenance/migrations/__init__.py | 0 maintenance/models.py | 62 +++++++++++++++++++ maintenance/tests.py | 3 + maintenance/translation.py | 8 +++ maintenance/views.py | 3 + resources/admin/__init__.py | 30 +-------- resources/api/__init__.py | 1 - resources/api/reservation.py | 7 +++ resources/api/resource.py | 16 ++++- .../migrations/0148_add_maintenance_mode.py | 35 +++++++++++ .../migrations/0149_create_maintenance_app.py | 31 ++++++++++ resources/models/__init__.py | 5 +- resources/models/resource.py | 32 ---------- resources/tests/conftest.py | 10 ++- resources/tests/test_reservation_api.py | 21 ++++++- resources/tests/test_resource_api.py | 39 +++++++++++- resources/translation.py | 8 +-- respa/settings.py | 1 + respa/urls.py | 1 + 25 files changed, 354 insertions(+), 82 deletions(-) create mode 100644 maintenance/__init__.py create mode 100644 maintenance/admin.py create mode 100644 maintenance/api/__init__.py rename {resources => maintenance}/api/announcements.py (80%) create mode 100644 maintenance/apps.py create mode 100644 maintenance/migrations/0001_create_maintenance_app.py create mode 100644 maintenance/migrations/__init__.py create mode 100644 maintenance/models.py create mode 100644 maintenance/tests.py create mode 100644 maintenance/translation.py create mode 100644 maintenance/views.py create mode 100644 resources/migrations/0148_add_maintenance_mode.py create mode 100644 resources/migrations/0149_create_maintenance_app.py diff --git a/maintenance/__init__.py b/maintenance/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/maintenance/admin.py b/maintenance/admin.py new file mode 100644 index 000000000..bee1529fe --- /dev/null +++ b/maintenance/admin.py @@ -0,0 +1,55 @@ +from django import forms +from django.contrib import admin +from django.db.models import Q +from django.contrib.admin import site as admin_site +from django.core.exceptions import ValidationError +from django.utils.translation import gettext_lazy as _ + +from modeltranslation.admin import TranslationAdmin + + +from .models import MaintenanceMessage, MaintenanceMode + + +class MaintenanceModeAdmin(admin.ModelAdmin): + pass + +class MaintenanceModeInline(admin.TabularInline): + model = MaintenanceMode + fields = ('start', 'end', ) + verbose_name = _('maintenance mode') + verbose_name_plural = _('maintenance modes') + extra = 0 + + +class MaintenanceMessageAdminForm(forms.ModelForm): + class Meta: + model = MaintenanceMessage + fields = ('start', 'end', 'message', ) + + def clean(self): + start = self.cleaned_data['start'] + end = self.cleaned_data['end'] + query = Q(end__gt=start, start__lt=end) + if self.instance and self.instance.pk: + query &= ~Q(pk=self.instance.pk) + collision = MaintenanceMessage.objects.filter(query) + if collision.exists(): + raise ValidationError(_('maintenance message already exists.')) + +class MaintenanceMessageAdmin(TranslationAdmin): + form = MaintenanceMessageAdminForm + inlines = ( MaintenanceModeInline, ) + fieldsets = ( + (_('General'), { + 'fields': ( + 'start', + 'end', + 'message' + ), + }), + ) + + +admin_site.register(MaintenanceMessage, MaintenanceMessageAdmin) +admin_site.register(MaintenanceMode, MaintenanceModeAdmin) \ No newline at end of file diff --git a/maintenance/api/__init__.py b/maintenance/api/__init__.py new file mode 100644 index 000000000..5f0a45736 --- /dev/null +++ b/maintenance/api/__init__.py @@ -0,0 +1 @@ +from .announcements import MaintenanceMessageViewSet \ No newline at end of file diff --git a/resources/api/announcements.py b/maintenance/api/announcements.py similarity index 80% rename from resources/api/announcements.py rename to maintenance/api/announcements.py index f84bd974f..5bd1dba34 100644 --- a/resources/api/announcements.py +++ b/maintenance/api/announcements.py @@ -1,6 +1,6 @@ from rest_framework import viewsets -from .base import TranslatedModelSerializer, register_view -from resources.models import MaintenanceMessage +from resources.api.base import TranslatedModelSerializer, register_view +from maintenance.models import MaintenanceMessage diff --git a/maintenance/apps.py b/maintenance/apps.py new file mode 100644 index 000000000..df4a3eb8d --- /dev/null +++ b/maintenance/apps.py @@ -0,0 +1,6 @@ +from django.apps import AppConfig + + +class MaintenanceConfig(AppConfig): + default_auto_field = 'django.db.models.BigAutoField' + name = 'maintenance' diff --git a/maintenance/migrations/0001_create_maintenance_app.py b/maintenance/migrations/0001_create_maintenance_app.py new file mode 100644 index 000000000..1536110d7 --- /dev/null +++ b/maintenance/migrations/0001_create_maintenance_app.py @@ -0,0 +1,57 @@ +# Generated by Django 3.2.19 on 2023-09-04 06:19 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='MaintenanceMessage', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created_at', models.DateTimeField(default=django.utils.timezone.now, verbose_name='Time of creation')), + ('modified_at', models.DateTimeField(default=django.utils.timezone.now, verbose_name='Time of modification')), + ('message', models.TextField(verbose_name='Message')), + ('message_fi', models.TextField(null=True, verbose_name='Message')), + ('message_en', models.TextField(null=True, verbose_name='Message')), + ('message_sv', models.TextField(null=True, verbose_name='Message')), + ('start', models.DateTimeField(verbose_name='Begin time')), + ('end', models.DateTimeField(verbose_name='End time')), + ('created_by', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='maintenancemessage_created', to=settings.AUTH_USER_MODEL, verbose_name='Created by')), + ('modified_by', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='maintenancemessage_modified', to=settings.AUTH_USER_MODEL, verbose_name='Modified by')), + ], + options={ + 'verbose_name': 'maintenance message', + 'verbose_name_plural': 'maintenance messages', + 'ordering': ('start',), + }, + ), + migrations.CreateModel( + name='MaintenanceMode', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created_at', models.DateTimeField(default=django.utils.timezone.now, verbose_name='Time of creation')), + ('modified_at', models.DateTimeField(default=django.utils.timezone.now, verbose_name='Time of modification')), + ('start', models.DateTimeField(verbose_name='Begin time')), + ('end', models.DateTimeField(verbose_name='End time')), + ('created_by', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='maintenancemode_created', to=settings.AUTH_USER_MODEL, verbose_name='Created by')), + ('maintenance_message', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='maintenance.maintenancemessage')), + ('modified_by', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='maintenancemode_modified', to=settings.AUTH_USER_MODEL, verbose_name='Modified by')), + ], + options={ + 'verbose_name': 'maintenance mode', + 'verbose_name_plural': 'maintenance modes', + 'ordering': ('start',), + }, + ), + ] diff --git a/maintenance/migrations/__init__.py b/maintenance/migrations/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/maintenance/models.py b/maintenance/models.py new file mode 100644 index 000000000..723515c07 --- /dev/null +++ b/maintenance/models.py @@ -0,0 +1,62 @@ +import datetime + + +from django.db import models +from django.utils import timezone +from django.utils.translation import gettext_lazy as _ +from django.core.exceptions import ValidationError + + +from resources.models.base import ModifiableModel + + +class MaintenanceMessageQuerySet(models.QuerySet): + def active(self): + return self.filter(start__lt=timezone.now(), end__gt=timezone.now()) + +class MaintenanceMessage(ModifiableModel): + message = models.TextField(verbose_name=_('Message'), null=False, blank=False) + start = models.DateTimeField(verbose_name=_('Begin time'), null=False, blank=False) + end = models.DateTimeField(verbose_name=_('End time'), null=False, blank=False) + + + objects = MaintenanceMessageQuerySet.as_manager() + class Meta: + verbose_name = _('maintenance message') + verbose_name_plural = _('maintenance messages') + ordering = ('start', ) + + + def __str__(self): + return f"{_('maintenance message')} \ + {timezone.localtime(self.start).replace(tzinfo=None)} - \ + {timezone.localtime(self.end).replace(tzinfo=None)}" \ + .capitalize() + + + def clean(self): + super().clean() + if self.end <= self.start: + raise ValidationError(_("Invalid start or end time")) + +class MaintenanceModeQuerySet(models.QuerySet): + def active(self): + return self.filter(start__lt=timezone.now(), end__gt=timezone.now()) + + +class MaintenanceMode(ModifiableModel): + start = models.DateTimeField(verbose_name=_('Begin time'), null=False, blank=False) + end = models.DateTimeField(verbose_name=_('End time'), null=False, blank=False) + maintenance_message = models.ForeignKey(MaintenanceMessage, on_delete=models.CASCADE, null=True, blank=True) + + objects = MaintenanceModeQuerySet.as_manager() + + class Meta: + verbose_name = _('maintenance mode') + verbose_name_plural = _('maintenance modes') + ordering = ('start', ) + + def clean(self): + super().clean() + if self.end <= self.start: + raise ValidationError(_("Invalid start or end time")) diff --git a/maintenance/tests.py b/maintenance/tests.py new file mode 100644 index 000000000..7ce503c2d --- /dev/null +++ b/maintenance/tests.py @@ -0,0 +1,3 @@ +from django.test import TestCase + +# Create your tests here. diff --git a/maintenance/translation.py b/maintenance/translation.py new file mode 100644 index 000000000..1424dcdd0 --- /dev/null +++ b/maintenance/translation.py @@ -0,0 +1,8 @@ +from modeltranslation.translator import TranslationOptions, register +from .models import MaintenanceMessage + + +@register(MaintenanceMessage) +class MaintenanceMessageTranslationOptions(TranslationOptions): + fields = ('message', ) + required_languages = ('fi', 'en', 'sv', ) diff --git a/maintenance/views.py b/maintenance/views.py new file mode 100644 index 000000000..91ea44a21 --- /dev/null +++ b/maintenance/views.py @@ -0,0 +1,3 @@ +from django.shortcuts import render + +# Create your views here. diff --git a/resources/admin/__init__.py b/resources/admin/__init__.py index 4790b52ad..ccefd97bc 100644 --- a/resources/admin/__init__.py +++ b/resources/admin/__init__.py @@ -6,6 +6,7 @@ from django.contrib.admin import site as admin_site from django.contrib.admin.utils import unquote from django.contrib.admin.widgets import FilteredSelectMultiple +from django.contrib.admin.options import InlineModelAdmin from django.contrib.auth import get_user_model from django.contrib.auth.models import Group from django.contrib.gis.admin import OSMGeoAdmin @@ -29,7 +30,7 @@ ReservationHomeMunicipalityField, ReservationHomeMunicipalitySet, Resource, ResourceTag, ResourceAccessibility, ResourceEquipment, ResourceGroup, ResourceImage, ResourceType, TermsOfUse, Unit, UnitAuthorization, UnitIdentifier, UnitGroup, UnitGroupAuthorization, - MaintenanceMessage, UniversalFormFieldType, ResourceUniversalField, ResourceUniversalFormOption, + UniversalFormFieldType, ResourceUniversalField, ResourceUniversalFormOption ) from ..models.utils import generate_id from munigeo.models import Municipality @@ -529,32 +530,6 @@ class RespaTokenAdmin(admin.ModelAdmin): raw_id_fields = ('user',) -class MaintenanceMessageAdminForm(forms.ModelForm): - class Meta: - model = MaintenanceMessage - fields = ('start', 'end', 'message', ) - - def clean(self): - start = self.cleaned_data['start'] - end = self.cleaned_data['end'] - query = Q(end__gt=start, start__lt=end) - if self.instance and self.instance.pk: - query &= ~Q(pk=self.instance.pk) - collision = MaintenanceMessage.objects.filter(query) - if collision.exists(): - raise ValidationError(_('maintenance message already exists.')) - -class MaintenanceMessageAdmin(TranslationAdmin): - form = MaintenanceMessageAdminForm - fieldsets = ( - (_('General'), { - 'fields': ( - 'start', - 'end', - 'message' - ), - }), - ) admin_site.register(ResourceImage, ResourceImageAdmin) admin_site.register(Resource, ResourceAdmin) @@ -584,6 +559,5 @@ class MaintenanceMessageAdmin(TranslationAdmin): if admin.site.is_registered(Token): admin.site.unregister(Token) admin_site.register(Token, RespaTokenAdmin) -admin_site.register(MaintenanceMessage, MaintenanceMessageAdmin) admin_site.register(UniversalFormFieldType, UniversalFieldAdmin) admin_site.register(ResourceUniversalFormOption, ResourceUniversalFormOptionAdmin) diff --git a/resources/api/__init__.py b/resources/api/__init__.py index 2decdc46c..205030b88 100644 --- a/resources/api/__init__.py +++ b/resources/api/__init__.py @@ -9,7 +9,6 @@ from .unit import UnitViewSet from .search import TypeaheadViewSet from .equipment import EquipmentViewSet -from .announcements import MaintenanceMessageViewSet from rest_framework import routers diff --git a/resources/api/reservation.py b/resources/api/reservation.py index 8dd66b642..649aed75b 100644 --- a/resources/api/reservation.py +++ b/resources/api/reservation.py @@ -54,6 +54,8 @@ from respa.renderers import ResourcesBrowsableAPIRenderer from payments.utils import is_free, get_price +from maintenance.models import MaintenanceMode + User = get_user_model() # FIXME: Make this configurable? @@ -266,6 +268,11 @@ def validate(self, data): obj_user_is_staff = bool(request_user and request_user.is_staff) + if (not reservation or (reservation and reservation.state != Reservation.WAITING_FOR_PAYMENT)) \ + and MaintenanceMode.objects.active().exists(): + raise ValidationError(_('Reservations are disabled at this moment.')) + + try: resource = data['resource'] except KeyError: diff --git a/resources/api/resource.py b/resources/api/resource.py index 18b212a3b..5aeb8e829 100644 --- a/resources/api/resource.py +++ b/resources/api/resource.py @@ -65,6 +65,7 @@ from rest_framework.settings import api_settings as drf_settings from rest_framework.relations import PrimaryKeyRelatedField from resources.models.utils import log_entry +from maintenance.models import MaintenanceMode from drf_yasg import openapi from drf_yasg.utils import swagger_auto_schema @@ -615,6 +616,7 @@ class ResourceSerializer(ExtraDataMixin, TranslatedModelSerializer, munigeo_api. resource_staff_emails = ResourceStaffEmailsField() universal_field = ResourceUniversalFieldSerializer(many=True, read_only=True, source='resource_universal_field') reservable_by_all_staff = serializers.BooleanField(required=False) + reservable = serializers.SerializerMethodField() def get_max_price_per_hour(self, obj): """Backwards compatibility for 'max_price_per_hour' field that is now deprecated""" @@ -662,7 +664,8 @@ def get_user_permissions(self, obj): user = prefetched_user or request.user can_make_reservations_for_customers = obj.can_create_reservations_for_other_users(user) if request else False - return { + + permissions = { 'can_make_reservations': obj.can_make_reservations(user) if request else False, **({'can_make_reservations_for_customer': can_make_reservations_for_customers} if (request and can_make_reservations_for_customers) else {}), 'can_ignore_opening_hours': obj.can_ignore_opening_hours(user) if request else False, @@ -672,6 +675,12 @@ def get_user_permissions(self, obj): 'can_bypass_payment': obj.can_bypass_payment(user) if request else False, } + + if MaintenanceMode.objects.active().exists(): + return permissions.fromkeys(permissions, False) + + return permissions + def get_is_favorite(self, obj): request = self.context.get('request', None) return request.user in obj.favorited_by.all() @@ -684,6 +693,11 @@ def get_payment_terms(self, obj): data = TermsOfUseSerializer(obj.payment_terms).data return data['text'] + def get_reservable(self, obj): + if MaintenanceMode.objects.active().exists(): + return False + return obj.reservable + def get_reservable_before(self, obj): request = self.context.get('request') prefetched_user = self.context.get('prefetched_user', None) diff --git a/resources/migrations/0148_add_maintenance_mode.py b/resources/migrations/0148_add_maintenance_mode.py new file mode 100644 index 000000000..fcdeaa266 --- /dev/null +++ b/resources/migrations/0148_add_maintenance_mode.py @@ -0,0 +1,35 @@ +# Generated by Django 3.2.19 on 2023-09-01 09:35 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('resources', '0147_reserver_id_only_business'), + ] + + operations = [ + migrations.CreateModel( + name='MaintenanceMode', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created_at', models.DateTimeField(default=django.utils.timezone.now, verbose_name='Time of creation')), + ('modified_at', models.DateTimeField(default=django.utils.timezone.now, verbose_name='Time of modification')), + ('start', models.DateTimeField(verbose_name='Begin time')), + ('end', models.DateTimeField(verbose_name='End time')), + ('created_by', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='maintenancemode_created', to=settings.AUTH_USER_MODEL, verbose_name='Created by')), + ('maintenance_message', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='resources.maintenancemessage')), + ('modified_by', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='maintenancemode_modified', to=settings.AUTH_USER_MODEL, verbose_name='Modified by')), + ], + options={ + 'verbose_name': 'maintenance mode', + 'verbose_name_plural': 'maintenance modes', + 'ordering': ('start',), + }, + ), + ] diff --git a/resources/migrations/0149_create_maintenance_app.py b/resources/migrations/0149_create_maintenance_app.py new file mode 100644 index 000000000..06cd52146 --- /dev/null +++ b/resources/migrations/0149_create_maintenance_app.py @@ -0,0 +1,31 @@ +# Generated by Django 3.2.19 on 2023-09-04 06:19 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('resources', '0148_add_maintenance_mode'), + ] + + operations = [ + migrations.RemoveField( + model_name='maintenancemode', + name='created_by', + ), + migrations.RemoveField( + model_name='maintenancemode', + name='maintenance_message', + ), + migrations.RemoveField( + model_name='maintenancemode', + name='modified_by', + ), + migrations.DeleteModel( + name='MaintenanceMessage', + ), + migrations.DeleteModel( + name='MaintenanceMode', + ), + ] diff --git a/resources/models/__init__.py b/resources/models/__init__.py index a92dd4103..f1f92984b 100644 --- a/resources/models/__init__.py +++ b/resources/models/__init__.py @@ -7,8 +7,8 @@ ) from .resource import ( Purpose, Resource, ResourceType, ResourceImage, ResourceEquipment, ResourceGroup, - ResourceDailyOpeningHours, TermsOfUse, ResourceTag, MaintenanceMessage, ResourceUniversalField, - ResourceUniversalFormOption, + ResourceDailyOpeningHours, TermsOfUse, ResourceTag, ResourceUniversalField, + ResourceUniversalFormOption ) from .equipment import Equipment, EquipmentAlias, EquipmentCategory from .unit import Unit, UnitAuthorization, UnitIdentifier @@ -52,7 +52,6 @@ 'UnitIdentifier', 'get_opening_hours', 'TimmiPayload', - 'MaintenanceMessage', 'UniversalFormFieldType', 'ResourceUniversalField', 'ResourceUniversalFormOption', diff --git a/resources/models/resource.py b/resources/models/resource.py index 3a99e7b28..ec034f323 100644 --- a/resources/models/resource.py +++ b/resources/models/resource.py @@ -1174,35 +1174,3 @@ def __str__(self): lower = self.open_between.lower upper = self.open_between.upper return "%s: %s -> %s" % (self.resource, lower, upper) - - -class MaintenanceMessageQuerySet(models.QuerySet): - def active(self): - return self.filter(start__lt=timezone.now(), end__gt=timezone.now()) - -class MaintenanceMessage(ModifiableModel): - message = models.TextField(verbose_name=_('Message'), null=False, blank=False) - start = models.DateTimeField(verbose_name=_('Begin time'), null=False, blank=False) - end = models.DateTimeField(verbose_name=_('End time'), null=False, blank=False) - - - objects = MaintenanceMessageQuerySet.as_manager() - class Meta: - verbose_name = _('maintenance message') - verbose_name_plural = _('maintenance messages') - ordering = ('start', ) - - - def __str__(self): - return f"{_('maintenance message')} \ - {timezone.localtime(self.start).replace(tzinfo=None)} - \ - {timezone.localtime(self.end).replace(tzinfo=None)}" \ - .capitalize() - - - def clean(self): - super().clean() - if self.end <= self.start: - raise ValidationError(_("Invalid start or end time")) - - diff --git a/resources/tests/conftest.py b/resources/tests/conftest.py index d9e14773f..de7276868 100644 --- a/resources/tests/conftest.py +++ b/resources/tests/conftest.py @@ -9,11 +9,11 @@ from resources.enums import UnitAuthorizationLevel from resources.models import Resource, ResourceType, Unit, Purpose, Day, Period from resources.models import Equipment, EquipmentAlias, ResourceEquipment, EquipmentCategory, TermsOfUse, ResourceGroup -from resources.models import AccessibilityValue, AccessibilityViewpoint, ResourceAccessibility, UnitAccessibility, MaintenanceMessage +from resources.models import AccessibilityValue, AccessibilityViewpoint, ResourceAccessibility, UnitAccessibility from resources.models import ResourceUniversalFormOption, ResourceUniversalField, UniversalFormFieldType from resources.models import ReservationMetadataSet, ReservationMetadataField from munigeo.models import Municipality - +from maintenance.models import MaintenanceMessage, MaintenanceMode @pytest.fixture def api_client(): @@ -586,6 +586,12 @@ def maintenance_message(): message_sv='Detta är ett meddelande' ) +@pytest.fixture +def maintenance_mode(): + return MaintenanceMode.objects.create( + start=timezone.now(), end=timezone.now() + datetime.timedelta(minutes=20) + ) + @pytest.mark.django_db @pytest.fixture def universal_form_field_type(): diff --git a/resources/tests/test_reservation_api.py b/resources/tests/test_reservation_api.py index 3ef16c687..74d6ec4ee 100644 --- a/resources/tests/test_reservation_api.py +++ b/resources/tests/test_reservation_api.py @@ -15,11 +15,17 @@ from caterings.models import CateringOrder, CateringProvider from resources.enums import UnitAuthorizationLevel -from resources.models import (Period, Day, Reservation, Resource, ResourceGroup, ReservationMetadataField, - ReservationMetadataSet, UnitAuthorization) +from resources.models import ( + Period, Day, Reservation, + Resource, ResourceGroup, ReservationMetadataField, + ReservationMetadataSet, UnitAuthorization +) from notifications.models import NotificationTemplate, NotificationType from notifications.tests.utils import check_received_mail_exists -from .utils import check_disallowed_methods, assert_non_field_errors_contain, assert_response_objects, MAX_QUERIES +from .utils import ( + check_disallowed_methods, assert_non_field_errors_contain, + assert_response_objects, MAX_QUERIES +) DEFAULT_RESERVATION_EXTRA_FIELDS = ('reserver_name', 'reserver_phone_number', 'reserver_address_street', @@ -3258,3 +3264,12 @@ def test_reservations_made_to_unit_with_overlap_restriction_with_different_times assert response.status_code == expected if expected == 400: assert response.data.get('non_field_errors')[0].code == 'conflicting_reservation' + +@pytest.mark.django_db +def test_reservation_not_allowed_during_maintenance_mode( + api_client, user, list_url, reservation_data_extra, maintenance_mode): + api_client.force_authenticate(user=user) + + response = api_client.post(list_url, data=reservation_data_extra) + assert response.status_code == 400 + assert_non_field_errors_contain(response, 'Reservations are disabled at this moment') \ No newline at end of file diff --git a/resources/tests/test_resource_api.py b/resources/tests/test_resource_api.py index c6c24917e..445b880ad 100644 --- a/resources/tests/test_resource_api.py +++ b/resources/tests/test_resource_api.py @@ -67,10 +67,9 @@ def _check_permissions_dict(api_client, resource, is_admin, is_manager, is_viewe url = reverse('resource-detail', kwargs={'pk': resource.pk}) response = api_client.get(url) - print(response.data) assert response.status_code == 200 permissions = response.data['user_permissions'] - + if can_create_reservations_for_other_users: # exists and is True if user is staff for the resource # OR the user is staff and the resource has reservable_by_all_staff set to True @@ -1264,4 +1263,38 @@ def test_user_permissions_external_resources(api_client, resource_in_unit, user, # can_create_reservations_for_other_users should be False for resource_in_unit3 _check_permissions_dict(api_client, resource_in_unit3, is_admin=False, is_manager=False, is_viewer=False, can_make_reservations=True, can_ignore_opening_hours=False, - can_bypass_payment=False, can_create_reservations_for_other_users=False) \ No newline at end of file + can_bypass_payment=False, can_create_reservations_for_other_users=False) + + +@pytest.mark.django_db +def test_user_permissions_in_resource_endpoint_during_maintenance_mode( + maintenance_mode, resource_in_unit, + user, api_client, + staff_user, staff_api_client): + resource_in_unit.unit.create_authorization(staff_user, 'manager') + url = reverse('resource-detail', kwargs={'pk': resource_in_unit.pk}) + + expected = { + 'can_bypass_payment': False, + 'can_ignore_opening_hours': False, + 'can_make_reservations': False, + 'is_admin': False, + 'is_manager': False, + 'is_viewer': False + } + + + api_client.force_authenticate(user=user) + response = api_client.get(url) + assert response.status_code == 200 + assert response.data['user_permissions'] == expected + + + expected.update({ + 'can_make_reservations_for_customer': False + }) + + staff_api_client.force_authenticate(user=staff_user) + response = staff_api_client.get(url) + assert response.status_code == 200 + assert response.data['user_permissions'] == expected diff --git a/resources/translation.py b/resources/translation.py index 26d013024..5ee7424c5 100644 --- a/resources/translation.py +++ b/resources/translation.py @@ -5,7 +5,7 @@ Purpose, Resource, ResourceEquipment, ResourceImage, ResourceType, TermsOfUse, Unit, UnitGroup, Reservation, ReservationHomeMunicipalityField, - MaintenanceMessage, UniversalFormFieldType, ResourceUniversalField, + UniversalFormFieldType, ResourceUniversalField, ResourceUniversalFormOption, ) @@ -75,12 +75,6 @@ class ReservationHomeMunicipalityFieldTranslationOptions(TranslationOptions): fields = ('name',) required_languages = ('fi', 'en', 'sv') -@register(MaintenanceMessage) -class MaintenanceMessageTranslationOptions(TranslationOptions): - fields = ('message', ) - required_languages = ('fi', 'en', 'sv', ) - - @register(UniversalFormFieldType) class UniversalFormFieldTranslationOptions(TranslationOptions): pass diff --git a/respa/settings.py b/respa/settings.py index 9fb94f489..ee91e067d 100644 --- a/respa/settings.py +++ b/respa/settings.py @@ -222,6 +222,7 @@ 'respa_outlook', 'respa_o365', 'respa_admin', + 'maintenance', 'sanitized_dump', 'drf_yasg', diff --git a/respa/urls.py b/respa/urls.py index 3c8d9a152..c0596c840 100644 --- a/respa/urls.py +++ b/respa/urls.py @@ -10,6 +10,7 @@ from resources.views.ical import ICalFeedView import accessibility.api +import maintenance.api if getattr(settings, 'RESPA_COMMENTS_ENABLED', False): import comments.api From 65a1c4a13b94ed44c6813bc350b8f8621d8ef8ee Mon Sep 17 00:00:00 2001 From: SanttuA Date: Tue, 5 Sep 2023 12:50:58 +0300 Subject: [PATCH 08/13] Added validation for notification jinja templates (#282) Changes: - Notification templates validate that the `short_message`, `body` and `html_body` are valid jinja templates --- notifications/models.py | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/notifications/models.py b/notifications/models.py index 333af9c5c..0746d8280 100644 --- a/notifications/models.py +++ b/notifications/models.py @@ -8,7 +8,7 @@ from django.utils.translation import gettext_lazy as _ from django.utils.formats import date_format from jinja2 import StrictUndefined -from jinja2.exceptions import TemplateError +from jinja2.exceptions import TemplateError, UndefinedError from jinja2.sandbox import SandboxedEnvironment from parler.models import TranslatableModel, TranslatedFields from parler.utils.context import switch_language @@ -183,6 +183,25 @@ def clean(self, **kwargs): elif NotificationTemplate.objects.filter(type=self.type, is_default_template=False).exists(): logger.info("New default template of type {} was created.".format(self.type)) + self.validate_templates() + + def validate_templates(self): + context = {} + env = SandboxedEnvironment(trim_blocks=True, lstrip_blocks=True, undefined=StrictUndefined) + env.filters['reservation_time'] = reservation_time + env.filters['format_datetime'] = format_datetime + env.filters['format_datetime_tz'] = format_datetime_tz + templates = ['short_message', 'body', 'html_body'] + for template in templates: + try: + env.from_string(getattr(self, template)).render(context) + except UndefinedError as e: + # context can have various variables that are hard to test without actual data + # so we just skip validation for them + continue + except TemplateError as e: + raise ValidationError({template: e}) + def reservation_time(res): if isinstance(res, dict): @@ -218,7 +237,7 @@ def render_notification_template(notification_type, context, language_code=DEFAU class NotificationTemplateGroup(ModifiableModel): identifier = models.CharField(verbose_name=_('Identifier'), max_length=100) name = models.CharField(verbose_name=_('Name'), max_length=200) - templates = models.ManyToManyField(NotificationTemplate, + templates = models.ManyToManyField(NotificationTemplate, verbose_name=_('Notification templates'), related_name='groups', blank=True, @@ -232,4 +251,3 @@ class Meta: def __str__(self): return self.name - \ No newline at end of file From 2ad57a61e2f18f27328b5e67ce028e4b13eb6788 Mon Sep 17 00:00:00 2001 From: SanttuA Date: Tue, 5 Sep 2023 13:20:59 +0300 Subject: [PATCH 09/13] Fixed RA user can approve reservation issues (#280) Changes: - it is no longer assumed user has `can_approve_reservation` permission when they have unit manager permissions - when all unit's permissions get deleted, `can_approve_reservation` is also removed from user if the user had it - when clicking to toggle `can_approve_reservation`, all checkboxes belonging to the same unit, will change to the same value --- respa_admin/forms.py | 5 ++++- respa_admin/static_src/js/userForm.js | 18 +++++++++++++++++- .../respa_admin/resources/_unit_user_list.html | 2 +- respa_admin/templatetags/templatetags.py | 4 ++-- 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/respa_admin/forms.py b/respa_admin/forms.py index 32edd63dd..b19afdf6d 100644 --- a/respa_admin/forms.py +++ b/respa_admin/forms.py @@ -733,12 +733,13 @@ def __init__(self, *args, **kwargs): user_has_unit_group_auth = self.request.user.unit_group_authorizations.to_unit(unit).admin_level().exists() can_approve_initial_value = permission_checker.has_perm( "unit:can_approve_reservation", self.instance.subject - ) or self.instance.subject.is_manager(self.instance.authorized) + ) if not user_has_unit_auth and not user_has_unit_group_auth: self.fields['subject'].disabled = True self.fields['level'].disabled = True self.fields['can_approve_reservation'].disabled = True self.is_disabled = True + self.fields['can_approve_reservation'].widget.attrs['data-unit-id'] = f'{unit.id}' self.fields['can_approve_reservation'].initial = can_approve_initial_value def clean(self): @@ -750,6 +751,8 @@ def clean(self): if not user_has_unit_auth and not user_has_unit_group_auth: self.add_error('subject', _('You can\'t add, change or delete permissions to unit you are not admin of')) self.cleaned_data[DELETION_FIELD_NAME] = False + if self.cleaned_data[DELETION_FIELD_NAME]: + self.cleaned_data['can_approve_reservation'] = False return cleaned_data class Meta: diff --git a/respa_admin/static_src/js/userForm.js b/respa_admin/static_src/js/userForm.js index 17bab15b2..94585f9e5 100644 --- a/respa_admin/static_src/js/userForm.js +++ b/respa_admin/static_src/js/userForm.js @@ -103,9 +103,25 @@ function updateAllPermissionMgmtFormIndices() { } } +function canApproveReservationsListener() { + $('[id*="-can_approve_reservation"]').on('click', handleCanApproveReservationsChange); +} + +function handleCanApproveReservationsChange(event) { + const unitId = $(this).closest('.custom-checkbox').attr('data-unit-id') + const value = event.currentTarget.checked + if (unitId) { + const $inputs = $('[data-unit-id="' + unitId + '"]'); + $inputs.prop('checked', value) + } + + initializeUserForm(); +} + export function initializeUserFormEventHandlers() { enableRemovePermission(); enableAddNewPermission(); setEmptyPermissionItem(); isStaffCheckboxListener(); -} \ No newline at end of file + canApproveReservationsListener(); +} diff --git a/respa_admin/templates/respa_admin/resources/_unit_user_list.html b/respa_admin/templates/respa_admin/resources/_unit_user_list.html index f02f59021..e29973ec7 100644 --- a/respa_admin/templates/respa_admin/resources/_unit_user_list.html +++ b/respa_admin/templates/respa_admin/resources/_unit_user_list.html @@ -40,7 +40,7 @@

{% trans 'Unit management' %}

{% if unit_auths|length > 6 %}style="min-height: 690px;"{% endif %}> {% for authorization in unit_auths %} {% with authorization.authorized as unit_user %} - {% user_has_permission unit_user 'can_approve_reservation' unit as user_can_approve %} + {% user_has_permission unit_user 'unit:can_approve_reservation' unit as user_can_approve %}
{{ unit_user.first_name }} {{ unit_user.last_name }} diff --git a/respa_admin/templatetags/templatetags.py b/respa_admin/templatetags/templatetags.py index 745c87df1..8d420afe2 100644 --- a/respa_admin/templatetags/templatetags.py +++ b/respa_admin/templatetags/templatetags.py @@ -41,7 +41,7 @@ def get_value_from_dict(dict_data, key): @register.simple_tag def user_has_permission(user, permission, obj): - return user.has_perm(permission, obj) or obj.is_manager(user) + return user.has_perm(permission, obj) @register.filter @@ -51,4 +51,4 @@ def is_truthy(collection): @register.filter def remove_empty(collection): - return [value for value in collection if bool(value)] \ No newline at end of file + return [value for value in collection if bool(value)] From 05f99eeaf521a8877970d0fbf644794f74b3f940 Mon Sep 17 00:00:00 2001 From: SanttuA Date: Fri, 8 Sep 2023 13:25:58 +0300 Subject: [PATCH 10/13] Added html preview to admin notification templates (#283) Changes: - admin notification template form has buttons to show and hide `html_body` preview iframe --- locale/en/LC_MESSAGES/django.po | 9 +++ locale/fi/LC_MESSAGES/django.po | 9 +++ locale/sv/LC_MESSAGES/django.po | 9 +++ notifications/admin.py | 1 + .../templates/admin/html_preview.html | 68 +++++++++++++++++++ 5 files changed, 96 insertions(+) create mode 100644 notifications/templates/admin/html_preview.html diff --git a/locale/en/LC_MESSAGES/django.po b/locale/en/LC_MESSAGES/django.po index 3d6a23936..4304d24a6 100644 --- a/locale/en/LC_MESSAGES/django.po +++ b/locale/en/LC_MESSAGES/django.po @@ -2800,3 +2800,12 @@ msgstr "" msgid "Update HTML Templates" msgstr "" + +msgid "HTML Preview" +msgstr "" + +msgid "Show Preview" +msgstr "" + +msgid "Hide Preview" +msgstr "" diff --git a/locale/fi/LC_MESSAGES/django.po b/locale/fi/LC_MESSAGES/django.po index 022617261..205523873 100644 --- a/locale/fi/LC_MESSAGES/django.po +++ b/locale/fi/LC_MESSAGES/django.po @@ -2177,3 +2177,12 @@ msgstr "sähköpostimalliprojekti" msgid "Update HTML Templates" msgstr "Päivitä HTML-pohjat" + +msgid "HTML Preview" +msgstr "HTML:n esikatselu" + +msgid "Show Preview" +msgstr "Näytä esikatselu" + +msgid "Hide Preview" +msgstr "Piilota esikatselu" diff --git a/locale/sv/LC_MESSAGES/django.po b/locale/sv/LC_MESSAGES/django.po index f6549637c..3fe6d635d 100644 --- a/locale/sv/LC_MESSAGES/django.po +++ b/locale/sv/LC_MESSAGES/django.po @@ -2115,3 +2115,12 @@ msgstr "e-postmallprojekt" msgid "Update HTML Templates" msgstr "Uppdatera HTML-mallar" + +msgid "HTML Preview" +msgstr "HTML-förhandsgranskning" + +msgid "Show Preview" +msgstr "Visa förhandsgranskning" + +msgid "Hide Preview" +msgstr "Dölj förhandsgranskningen" diff --git a/notifications/admin.py b/notifications/admin.py index c18a78534..a52f4a234 100644 --- a/notifications/admin.py +++ b/notifications/admin.py @@ -71,6 +71,7 @@ class NotificationTemplateAdmin(TranslatableAdmin): # variables are accessed? # form = NotificationTemplateForm + change_form_template = 'admin/html_preview.html' actions = ['update_notification_html_templates'] def get_urls(self): diff --git a/notifications/templates/admin/html_preview.html b/notifications/templates/admin/html_preview.html new file mode 100644 index 000000000..6c9e66d0b --- /dev/null +++ b/notifications/templates/admin/html_preview.html @@ -0,0 +1,68 @@ +{% extends "admin/parler/change_form.html" %} +{% load static %} +{% load i18n %} + +{% block extrahead %} + + +{% endblock %} + +{% block after_field_sets %}{{ block.super }} +
+

{% trans 'HTML Preview' %}

+ + +
+
+ +
+
+
+{% for item in original.items_set.all %} {{ item }} {% endfor %} +{% endblock %} From a8da00f2540bb9fe3454ddf1720096126f0e51e2 Mon Sep 17 00:00:00 2001 From: SanttuA Date: Thu, 14 Sep 2023 12:07:43 +0300 Subject: [PATCH 11/13] Added maintenance mode status to announcements api (#284) Changes: - announcements api will now include `is_maintenance_mode_on` which tells whether maintenance is currently on or not --- maintenance/api/announcements.py | 19 ++++++++++++++--- resources/tests/conftest.py | 5 +++-- resources/tests/test_maintenance_message.py | 23 ++++++++++++++++++++- 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/maintenance/api/announcements.py b/maintenance/api/announcements.py index 5bd1dba34..fe3ebf341 100644 --- a/maintenance/api/announcements.py +++ b/maintenance/api/announcements.py @@ -1,13 +1,26 @@ -from rest_framework import viewsets +from django.utils import timezone +from rest_framework import viewsets, serializers from resources.api.base import TranslatedModelSerializer, register_view from maintenance.models import MaintenanceMessage class MaintenanceMessageSerializer(TranslatedModelSerializer): + is_maintenance_mode_on = serializers.SerializerMethodField() + class Meta: model = MaintenanceMessage - fields = ('message', ) + fields = ('message', 'is_maintenance_mode_on') + + def get_is_maintenance_mode_on(self, obj): + maintenance_modes = obj.maintenancemode_set.all() + now = timezone.now() + + for maintenance_mode in maintenance_modes: + if maintenance_mode.start <= now <= maintenance_mode.end: + return True + + return False @@ -20,4 +33,4 @@ def get_queryset(self): return self.queryset.active() -register_view(MaintenanceMessageViewSet, 'announcements', base_name='announcements') \ No newline at end of file +register_view(MaintenanceMessageViewSet, 'announcements', base_name='announcements') diff --git a/resources/tests/conftest.py b/resources/tests/conftest.py index de7276868..5199ffba6 100644 --- a/resources/tests/conftest.py +++ b/resources/tests/conftest.py @@ -587,9 +587,10 @@ def maintenance_message(): ) @pytest.fixture -def maintenance_mode(): +def maintenance_mode(maintenance_message): return MaintenanceMode.objects.create( - start=timezone.now(), end=timezone.now() + datetime.timedelta(minutes=20) + start=timezone.now(), end=timezone.now() + datetime.timedelta(minutes=20), + maintenance_message=maintenance_message ) @pytest.mark.django_db diff --git a/resources/tests/test_maintenance_message.py b/resources/tests/test_maintenance_message.py index be280fc64..cc12e6224 100644 --- a/resources/tests/test_maintenance_message.py +++ b/resources/tests/test_maintenance_message.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- import pytest -import datetime +import datetime from django.utils import timezone from django.urls import reverse @@ -25,3 +25,24 @@ def test_maintenance_message(client, maintenance_message): results = response.data['results'] assert len(results) == 0 + +@pytest.mark.django_db +def test_maintenance_message_is_maintenance_mode_on_when_on(client, maintenance_message, maintenance_mode): + response = client.get(LIST_URL, HTTP_ACCEPT='text/html') + assert response.status_code == 200 + results = response.data['results'] + assert len(results) == 1 + message = results[0] + assert message != None + assert message['is_maintenance_mode_on'] == True + + +@pytest.mark.django_db +def test_maintenance_message_is_maintenance_mode_on_when_off(client, maintenance_message): + response = client.get(LIST_URL, HTTP_ACCEPT='text/html') + assert response.status_code == 200 + results = response.data['results'] + assert len(results) == 1 + message = results[0] + assert message != None + assert message['is_maintenance_mode_on'] == False From d758a2dbd39bf6fa2becb8d969d6667ec6770832 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 21 Sep 2023 21:04:34 +0000 Subject: [PATCH 12/13] Bump cryptography from 41.0.3 to 41.0.4 Bumps [cryptography](https://github.com/pyca/cryptography) from 41.0.3 to 41.0.4. - [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst) - [Commits](https://github.com/pyca/cryptography/compare/41.0.3...41.0.4) --- updated-dependencies: - dependency-name: cryptography dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index a509665ce..d2039c964 100644 --- a/requirements.txt +++ b/requirements.txt @@ -67,7 +67,7 @@ coverage[toml]==7.2.0 # via # -r requirements.in # pytest-cov -cryptography==41.0.3 +cryptography==41.0.4 # via # -r requirements.in # djangorestframework-simplejwt From ffa7aa8e145b20e7cf04ca500cd2d7603cacda5c Mon Sep 17 00:00:00 2001 From: SanttuA Date: Tue, 26 Sep 2023 12:40:35 +0300 Subject: [PATCH 13/13] Bump respa version to tku-v1.8 (#286) --- respa/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/respa/__init__.py b/respa/__init__.py index 21d4f84bc..043132123 100644 --- a/respa/__init__.py +++ b/respa/__init__.py @@ -1,3 +1,3 @@ -__version__ = 'tku-v1.7' +__version__ = 'tku-v1.8' VERSION = __version__