From 91dd67444f5139f85c91177382be2ed9e41ee917 Mon Sep 17 00:00:00 2001 From: Tobias Bengfort Date: Tue, 16 Nov 2021 13:56:07 +0100 Subject: [PATCH 1/6] mv annotate_end to model manager --- castellum/appointments/forms.py | 9 ++------- castellum/appointments/models.py | 11 +++++++++++ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/castellum/appointments/forms.py b/castellum/appointments/forms.py index 371b9c29a..83e6f6f70 100644 --- a/castellum/appointments/forms.py +++ b/castellum/appointments/forms.py @@ -23,13 +23,11 @@ import itertools from django import forms from django.conf import settings -from django.db import models from django.utils.translation import gettext_lazy as _ from castellum.pseudonyms.helpers import get_pseudonym from castellum.recruitment.models import Participation from castellum.utils import scheduler -from castellum.utils.expressions import MultiDurationExpr from castellum.utils.forms import DateTimeField from .models import MINUTE @@ -89,12 +87,9 @@ class AppointmentsForm(forms.ModelForm): break qs = Appointment.objects\ + .annotate_end()\ .exclude(participation=self.instance)\ - .filter(participation__status=Participation.INVITED)\ - .alias( - # corresponds to the Appointment.end property - end=MultiDurationExpr(models.F('start'), MINUTE, models.F('session__duration')), - ) + .filter(participation__status=Participation.INVITED) for session, key, start, end in appointments: if session.resource and qs.filter( diff --git a/castellum/appointments/models.py b/castellum/appointments/models.py index 9ed4c3559..c481adc8f 100644 --- a/castellum/appointments/models.py +++ b/castellum/appointments/models.py @@ -29,11 +29,20 @@ from django.utils.translation import gettext_lazy as _ from castellum.recruitment.models import Participation from castellum.studies.models import StudySession +from castellum.utils.expressions import MultiDurationExpr from castellum.utils.fields import DateTimeField MINUTE = datetime.timedelta(seconds=60) +class AppointmentQuerySet(models.QuerySet): + def annotate_end(self): + return self.annotate( + # corresponds to the Appointment.end property + end=MultiDurationExpr(models.F('start'), MINUTE, models.F('session__duration')), + ) + + class Appointment(models.Model): session = models.ForeignKey(StudySession, verbose_name=_('Session'), on_delete=models.CASCADE) start = DateTimeField(_('Start')) @@ -44,6 +53,8 @@ class Appointment(models.Model): verbose_name=_('Participation'), ) + objects = AppointmentQuerySet.as_manager() + class Meta: verbose_name = _('Appointment') ordering = ['start'] -- GitLab From 6826a7697d70fb20977244de88e227f9a49c4041 Mon Sep 17 00:00:00 2001 From: Tobias Bengfort Date: Tue, 16 Nov 2021 13:57:58 +0100 Subject: [PATCH 2/6] rm appointment validation --- castellum/appointments/forms.py | 36 --------------------------------- 1 file changed, 36 deletions(-) diff --git a/castellum/appointments/forms.py b/castellum/appointments/forms.py index 83e6f6f70..cbb1831a2 100644 --- a/castellum/appointments/forms.py +++ b/castellum/appointments/forms.py @@ -19,8 +19,6 @@ # License along with Castellum. If not, see # . -import itertools - from django import forms from django.conf import settings from django.utils.translation import gettext_lazy as _ @@ -30,9 +28,6 @@ from castellum.recruitment.models import Participation from castellum.utils import scheduler from castellum.utils.forms import DateTimeField -from .models import MINUTE -from .models import Appointment - class AppointmentsForm(forms.ModelForm): class Meta: @@ -68,37 +63,6 @@ class AppointmentsForm(forms.ModelForm): else: return '{} - {}'.format(session.name, duration) - def clean(self): - cleaned_data = super().clean() - - appointments = [] - for session in self.instance.study.studysession_set.all(): - key = 'appointment-{}'.format(session.pk) - start = cleaned_data.get(key) - if start: - appointments.append((session, key, start, start + session.duration * MINUTE)) - - for a, b in itertools.combinations(appointments, 2): - __, key1, start1, end1 = a - __, key2, start2, end2 = b - if start1 < end2 and start2 < end1: - self.add_error(key1, _('Appointments must not overlap.')) - self.add_error(key2, _('Appointments must not overlap.')) - break - - qs = Appointment.objects\ - .annotate_end()\ - .exclude(participation=self.instance)\ - .filter(participation__status=Participation.INVITED) - - for session, key, start, end in appointments: - if session.resource and qs.filter( - session__resource=session.resource, end__gt=start, start__lt=end, - ).exists(): - self.add_error(key, _('The required resource is not available at this time')) - - return cleaned_data - def save(self): pariticipation = super().save() self.appointment_changes = [] -- GitLab From f9f929346bbc5017da285d6a3d07b56c58cec160 Mon Sep 17 00:00:00 2001 From: Tobias Bengfort Date: Tue, 16 Nov 2021 13:58:11 +0100 Subject: [PATCH 3/6] add appointment warnings --- castellum/appointments/mixins.py | 30 +++++++++++++++++++ .../execution/views/test_appointments_view.py | 7 +++++ 2 files changed, 37 insertions(+) diff --git a/castellum/appointments/mixins.py b/castellum/appointments/mixins.py index 2440aef7e..1b793a0de 100644 --- a/castellum/appointments/mixins.py +++ b/castellum/appointments/mixins.py @@ -142,10 +142,40 @@ class BaseAppointmentsUpdateView(ParticipationMixin, PermissionRequiredMixin, Up def get_success_url(self): return self.request.path + def has_participation_overlap(self): + return any(( + self.object.appointment_set + .annotate_end() + .exclude(pk=appointment.pk) + .filter(end__gt=appointment.start, start__lt=appointment.end) + .exists() + ) for appointment in self.object.appointment_set.all()) + + def has_resource_overlap(self): + return any(appointment.session.resource and ( + Appointment.objects + .annotate_end() + .exclude(pk=appointment.pk) + .filter( + session__resource=appointment.session.resource, + participation__status=Participation.INVITED, + end__gt=appointment.start, + start__lt=appointment.end, + ) + .exists() + ) for appointment in self.object.appointment_set.select_related('session__resource')) + def form_valid(self, form): response = super().form_valid(form) send_appointment_notifications(self.request, self.object, form.appointment_changes) + messages.success(self.request, _('Data has been saved.')) + if self.has_participation_overlap(): + messages.warning(self.request, _('Some appointments overlap')) + if self.has_resource_overlap(): + messages.warning(self.request, _( + 'Some resources might not be available at the selected times' + )) return response diff --git a/tests/execution/views/test_appointments_view.py b/tests/execution/views/test_appointments_view.py index b3672da1f..b53e7b1ad 100644 --- a/tests/execution/views/test_appointments_view.py +++ b/tests/execution/views/test_appointments_view.py @@ -1,6 +1,7 @@ import datetime from django.contrib.auth.models import Permission +from django.contrib.messages import get_messages from django.utils import timezone import pytest @@ -77,6 +78,8 @@ def test_appointment_overlap(client, member, participation, time): 'appointment-{}_1'.format(session2.pk): time, }) assert response.status_code == 302 + messages = [str(m) for m in get_messages(response.wsgi_request)] + assert 'Some appointments overlap' not in messages @pytest.mark.parametrize('time', ( @@ -111,6 +114,8 @@ def test_appointment_resource_overlap(client, member, participation, time): 'appointment-{}_1'.format(session.pk): time, }) assert response.status_code == 302 + messages = [str(m) for m in get_messages(response.wsgi_request)] + assert 'Some resources might not be available at the selected times' not in messages def test_appointment_resource_overlap_ignore_not_invited(client, member, participation): @@ -136,6 +141,8 @@ def test_appointment_resource_overlap_ignore_not_invited(client, member, partici 'appointment-{}_1'.format(session.pk): '12:00', }) assert response.status_code == 302 + messages = [str(m) for m in get_messages(response.wsgi_request)] + assert 'Some resources might not be available at the selected times' not in messages def test_appointment_change_appointment_permission(client, member, participation): -- GitLab From e7d2e1cd2ace04f33f65b203bcacdeeb02439107 Mon Sep 17 00:00:00 2001 From: Tobias Bengfort Date: Tue, 16 Nov 2021 16:33:03 +0100 Subject: [PATCH 4/6] tweak wording --- castellum/appointments/mixins.py | 2 +- tests/execution/views/test_appointments_view.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/castellum/appointments/mixins.py b/castellum/appointments/mixins.py index 1b793a0de..189d2fa81 100644 --- a/castellum/appointments/mixins.py +++ b/castellum/appointments/mixins.py @@ -174,7 +174,7 @@ class BaseAppointmentsUpdateView(ParticipationMixin, PermissionRequiredMixin, Up messages.warning(self.request, _('Some appointments overlap')) if self.has_resource_overlap(): messages.warning(self.request, _( - 'Some resources might not be available at the selected times' + 'Some appointments for resources overlap' )) return response diff --git a/tests/execution/views/test_appointments_view.py b/tests/execution/views/test_appointments_view.py index b53e7b1ad..73b586b6b 100644 --- a/tests/execution/views/test_appointments_view.py +++ b/tests/execution/views/test_appointments_view.py @@ -115,7 +115,7 @@ def test_appointment_resource_overlap(client, member, participation, time): }) assert response.status_code == 302 messages = [str(m) for m in get_messages(response.wsgi_request)] - assert 'Some resources might not be available at the selected times' not in messages + assert 'Some appointments for resources overlap' not in messages def test_appointment_resource_overlap_ignore_not_invited(client, member, participation): @@ -142,7 +142,7 @@ def test_appointment_resource_overlap_ignore_not_invited(client, member, partici }) assert response.status_code == 302 messages = [str(m) for m in get_messages(response.wsgi_request)] - assert 'Some resources might not be available at the selected times' not in messages + assert 'Some appointments for resources overlap' not in messages def test_appointment_change_appointment_permission(client, member, participation): -- GitLab From e8b5308238c3555b5c9d5329f05ddc7d014d01fd Mon Sep 17 00:00:00 2001 From: Tobias Bengfort Date: Wed, 17 Nov 2021 18:02:53 +0100 Subject: [PATCH 5/6] mention specific resource in overlap warning --- castellum/appointments/mixins.py | 35 ++++++++++--------- .../execution/views/test_appointments_view.py | 8 ++--- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/castellum/appointments/mixins.py b/castellum/appointments/mixins.py index 189d2fa81..5e0eac7b9 100644 --- a/castellum/appointments/mixins.py +++ b/castellum/appointments/mixins.py @@ -37,6 +37,7 @@ from icalevents import icalparser from castellum.castellum_auth.mixins import PermissionRequiredMixin from castellum.recruitment.mixins import ParticipationMixin from castellum.recruitment.models import Participation +from castellum.studies.models import Resource from castellum.utils import add_working_days from castellum.utils import cached_request from castellum.utils import contrast_color @@ -151,19 +152,21 @@ class BaseAppointmentsUpdateView(ParticipationMixin, PermissionRequiredMixin, Up .exists() ) for appointment in self.object.appointment_set.all()) - def has_resource_overlap(self): - return any(appointment.session.resource and ( - Appointment.objects - .annotate_end() - .exclude(pk=appointment.pk) - .filter( - session__resource=appointment.session.resource, - participation__status=Participation.INVITED, - end__gt=appointment.start, - start__lt=appointment.end, - ) - .exists() - ) for appointment in self.object.appointment_set.select_related('session__resource')) + def get_resource_overlaps(self): + for resource in Resource.objects.all(): + if any(( + Appointment.objects + .annotate_end() + .exclude(pk=appointment.pk) + .filter( + session__resource=resource, + participation__status=Participation.INVITED, + end__gt=appointment.start, + start__lt=appointment.end, + ) + .exists() + ) for appointment in self.object.appointment_set.filter(session__resource=resource)): + yield resource def form_valid(self, form): response = super().form_valid(form) @@ -172,10 +175,10 @@ class BaseAppointmentsUpdateView(ParticipationMixin, PermissionRequiredMixin, Up messages.success(self.request, _('Data has been saved.')) if self.has_participation_overlap(): messages.warning(self.request, _('Some appointments overlap')) - if self.has_resource_overlap(): + for resource in self.get_resource_overlaps(): messages.warning(self.request, _( - 'Some appointments for resources overlap' - )) + 'Some appointments for {resource} overlap' + ).format(resource=resource)) return response diff --git a/tests/execution/views/test_appointments_view.py b/tests/execution/views/test_appointments_view.py index 73b586b6b..d47edaa49 100644 --- a/tests/execution/views/test_appointments_view.py +++ b/tests/execution/views/test_appointments_view.py @@ -92,7 +92,7 @@ def test_appointment_overlap(client, member, participation, time): '13:30', )) def test_appointment_resource_overlap(client, member, participation, time): - resource = baker.make(Resource) + resource = baker.make(Resource, name='MRI') session = baker.make( StudySession, study=participation.study, duration=60, resource=resource ) @@ -115,11 +115,11 @@ def test_appointment_resource_overlap(client, member, participation, time): }) assert response.status_code == 302 messages = [str(m) for m in get_messages(response.wsgi_request)] - assert 'Some appointments for resources overlap' not in messages + assert 'Some appointments for MRI overlap' not in messages def test_appointment_resource_overlap_ignore_not_invited(client, member, participation): - resource = baker.make(Resource) + resource = baker.make(Resource, name='MRI') session = baker.make( StudySession, study=participation.study, duration=60, resource=resource ) @@ -142,7 +142,7 @@ def test_appointment_resource_overlap_ignore_not_invited(client, member, partici }) assert response.status_code == 302 messages = [str(m) for m in get_messages(response.wsgi_request)] - assert 'Some appointments for resources overlap' not in messages + assert 'Some appointments for MRI overlap' not in messages def test_appointment_change_appointment_permission(client, member, participation): -- GitLab From 636d7f130b333051beb62763411b5502294a937a Mon Sep 17 00:00:00 2001 From: Tobias Bengfort Date: Wed, 17 Nov 2021 18:27:14 +0100 Subject: [PATCH 6/6] push computation to database --- castellum/appointments/mixins.py | 34 +++++++++++++++++++------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/castellum/appointments/mixins.py b/castellum/appointments/mixins.py index 5e0eac7b9..516937499 100644 --- a/castellum/appointments/mixins.py +++ b/castellum/appointments/mixins.py @@ -24,6 +24,7 @@ from urllib.parse import quote_plus from django.conf import settings from django.contrib import messages +from django.db import models from django.http import JsonResponse from django.urls import reverse from django.utils import formats @@ -153,20 +154,25 @@ class BaseAppointmentsUpdateView(ParticipationMixin, PermissionRequiredMixin, Up ) for appointment in self.object.appointment_set.all()) def get_resource_overlaps(self): - for resource in Resource.objects.all(): - if any(( - Appointment.objects - .annotate_end() - .exclude(pk=appointment.pk) - .filter( - session__resource=resource, - participation__status=Participation.INVITED, - end__gt=appointment.start, - start__lt=appointment.end, - ) - .exists() - ) for appointment in self.object.appointment_set.filter(session__resource=resource)): - yield resource + inner = ( + Appointment.objects + .annotate_end() + .exclude(pk=models.OuterRef('pk')) + .filter( + session__resource=models.OuterRef('session__resource'), + participation__status=Participation.INVITED, + end__gt=models.OuterRef('start'), + start__lt=models.OuterRef('end'), + ) + ) + outer = ( + self.object.appointment_set.all() + .annotate_end() + .alias(overlap=models.Subquery(inner.values('pk')[:1])) + .exclude(overlap=None) + .order_by() + ) + return Resource.objects.filter(pk__in=outer.values('session__resource__pk')) def form_valid(self, form): response = super().form_valid(form) -- GitLab