From 96dd9bb41384dbab1a72a02b26f60c657dfacebb Mon Sep 17 00:00:00 2001 From: Andreas Ellewsen <ae@uio.no> Date: Fri, 3 Dec 2021 15:34:06 +0100 Subject: [PATCH] Change rules for role dates Dates can now be changed after they have started and ended. This means that there is no situation which needs disabling the input fields, and disabling has been removed. Start and end date can now be in the past. The following rules apply: - Start dates can be any date in the past, and no more into the future than the max days property of the role type. - End dates follow the same rules - End dates must be equal to or later than start date. Notification publishing has been reviewed to ensure duplicate notifications are not created when start or end date is today. Resolve: GREG-148 --- .../sponsor/guest/guestRoleInfo/index.tsx | 4 -- .../sponsor/guest/newGuestRole/index.tsx | 2 - .../sponsor/register/stepPersonForm.tsx | 14 ++++++- greg/signals.py | 7 +++- gregui/api/serializers/role.py | 23 ----------- gregui/tests/api/serializers/test_role.py | 41 +++++-------------- 6 files changed, 30 insertions(+), 61 deletions(-) diff --git a/frontend/src/routes/sponsor/guest/guestRoleInfo/index.tsx b/frontend/src/routes/sponsor/guest/guestRoleInfo/index.tsx index 66112b9c..82c67530 100644 --- a/frontend/src/routes/sponsor/guest/guestRoleInfo/index.tsx +++ b/frontend/src/routes/sponsor/guest/guestRoleInfo/index.tsx @@ -138,10 +138,8 @@ export default function GuestRoleInfo({ guest }: GuestRoleInfoProps) { render={({ field: { onChange, value } }) => ( <DatePicker mask="____-__-__" - disabled={roleInfo.start_date <= today} label={t('input.roleStartDate')} value={value} - minDate={today} maxDate={todayPlusMaxDays} inputFormat="yyyy-MM-dd" onChange={onChange} @@ -159,8 +157,6 @@ export default function GuestRoleInfo({ guest }: GuestRoleInfoProps) { <DatePicker mask="____-__-__" label={t('input.roleEndDate')} - disabled={roleInfo.end_date < today} - minDate={today} maxDate={todayPlusMaxDays} value={value} inputFormat="yyyy-MM-dd" diff --git a/frontend/src/routes/sponsor/guest/newGuestRole/index.tsx b/frontend/src/routes/sponsor/guest/newGuestRole/index.tsx index f438dca6..bad293bd 100644 --- a/frontend/src/routes/sponsor/guest/newGuestRole/index.tsx +++ b/frontend/src/routes/sponsor/guest/newGuestRole/index.tsx @@ -212,7 +212,6 @@ function NewGuestRole({ guest, reloadGuestInfo }: NewGuestRoleProps) { label={t('input.roleStartDate')} disabled={!roleTypeChoice} value={field.value} - minDate={today} maxDate={todayPlusMaxDays()} inputFormat="yyyy-MM-dd" onChange={(value) => { @@ -232,7 +231,6 @@ function NewGuestRole({ guest, reloadGuestInfo }: NewGuestRoleProps) { label={t('input.roleEndDate')} disabled={!roleTypeChoice} value={field.value} - minDate={today} maxDate={todayPlusMaxDays()} inputFormat="yyyy-MM-dd" onChange={(value) => { diff --git a/frontend/src/routes/sponsor/register/stepPersonForm.tsx b/frontend/src/routes/sponsor/register/stepPersonForm.tsx index e796fa33..1bccad5a 100644 --- a/frontend/src/routes/sponsor/register/stepPersonForm.tsx +++ b/frontend/src/routes/sponsor/register/stepPersonForm.tsx @@ -19,6 +19,7 @@ import React, { useImperativeHandle, useState, } from 'react' +import { addDays } from 'date-fns/fp' import { useTranslation } from 'react-i18next' import { RegisterFormData } from './formData' import { PersonFormMethods } from './personFormMethods' @@ -44,9 +45,10 @@ const StepPersonForm = forwardRef( const [selectedRoleType, setSelectedRoleType] = useState( formData.role_type ? formData.role_type : '' ) + const today = new Date() + const [todayPlusMaxDays, setTodayPlusMaxDays] = useState(today) const roleTypes = useRoleTypes() const { displayContactAtUnit, displayComment } = useContext(FeatureContext) - const today: Date = new Date() const roleTypeSort = () => (a: RoleTypeData, b: RoleTypeData) => { if (i18n.language === 'en') { @@ -90,6 +92,14 @@ const StepPersonForm = forwardRef( const handleRoleTypeChange = (event: any) => { setValue('role_type', event.target.value) setSelectedRoleType(event.target.value) + const selectedRoleTypeInfo = roleTypes.find( + (rt) => rt.id === event.target.value + ) + if (selectedRoleTypeInfo === undefined) { + setTodayPlusMaxDays(today) + } else { + setTodayPlusMaxDays(addDays(selectedRoleTypeInfo.max_days)(today)) + } } function doSubmit() { @@ -210,6 +220,7 @@ const StepPersonForm = forwardRef( <DatePicker mask="____-__-__" label={t('input.roleStartDate')} + maxDate={todayPlusMaxDays} value={field.value} inputFormat="yyyy-MM-dd" onChange={(value) => { @@ -232,6 +243,7 @@ const StepPersonForm = forwardRef( <DatePicker mask="____-__-__" label={t('input.roleEndDate')} + maxDate={todayPlusMaxDays} value={field.value} inputFormat="yyyy-MM-dd" onChange={(value) => { diff --git a/greg/signals.py b/greg/signals.py index 5d427897..de65ab3e 100644 --- a/greg/signals.py +++ b/greg/signals.py @@ -134,9 +134,11 @@ def save_notification_callback(sender, instance, created, *args, **kwargs): return # Queue future notifications on start and end date for roles if isinstance(instance, Role) and hasattr(instance, "_changed_fields"): + today = datetime.date.today() if ( "start_date" in instance._changed_fields # pylint: disable=protected-access and instance.start_date + and instance.start_date != today ): Schedule.objects.create( func="greg.signals._queue_role_start_notification", @@ -144,7 +146,10 @@ def save_notification_callback(sender, instance, created, *args, **kwargs): next_run=date_to_datetime_midnight(instance.start_date), schedule_type=Schedule.ONCE, ) - if "end_date" in instance._changed_fields: # pylint: disable=protected-access + if ( + "end_date" in instance._changed_fields # pylint: disable=protected-access + and instance.end_date != today + ): Schedule.objects.create( func="greg.signals._queue_role_end_notification", args=f"{instance.id},True", diff --git a/gregui/api/serializers/role.py b/gregui/api/serializers/role.py index 0b867f09..9afc3922 100644 --- a/gregui/api/serializers/role.py +++ b/gregui/api/serializers/role.py @@ -28,29 +28,6 @@ class RoleSerializerUi(serializers.ModelSerializer): ) ] - def validate_start_date(self, start_date): - """Enfore rules for start_date. - - Must be present, can be blank, before today not allowed. - """ - if not start_date: - return start_date - today = datetime.date.today() - # New start dates cannot be in the past - if start_date < today: - raise serializers.ValidationError("Start date cannot be in the past") - - return start_date - - def validate_end_date(self, end_date): - """Ensure rules for end_date are followed""" - today = datetime.date.today() - if end_date < today: - raise serializers.ValidationError("End date cannot be in the past") - if self.instance and self.instance.end_date < today: - raise serializers.ValidationError("Role has ended, cannot change end date") - return end_date - def validate_orgunit(self, unit): """Enforce rules related to orgunit""" sponsor = self.context["sponsor"] diff --git a/gregui/tests/api/serializers/test_role.py b/gregui/tests/api/serializers/test_role.py index b36c2b7c..604fa4cc 100644 --- a/gregui/tests/api/serializers/test_role.py +++ b/gregui/tests/api/serializers/test_role.py @@ -25,8 +25,8 @@ def test_minimum_ok(role, sponsor_foo): @pytest.mark.django_db -def test_start_date_past_fail(role, sponsor_foo): - """Should fail because of start_date in the past""" +def test_start_date_past_ok(role, sponsor_foo): + """Should work even though start_date in the past""" ser = RoleSerializerUi( data={ "person": role.person.id, @@ -37,41 +37,29 @@ def test_start_date_past_fail(role, sponsor_foo): }, context={"sponsor": sponsor_foo}, ) - with pytest.raises( - ValidationError, - match=re.escape( - "{'start_date': [ErrorDetail(string='Start date cannot be in the past', code='invalid')]}" - ), - ): - ser.is_valid(raise_exception=True) + assert ser.is_valid(raise_exception=True) @pytest.mark.django_db -def test_end_date_past_fail(role, sponsor_foo): - """Should fail because of end_date in the past""" +def test_end_date_past_ok(role, sponsor_foo): + """Should work even though end_date in the past""" ser = RoleSerializerUi( data={ "person": role.person.id, "orgunit": role.orgunit.id, "type": role.type.id, - "start_date": datetime.date.today(), + "start_date": (timezone.now() - datetime.timedelta(days=12)).date(), "end_date": (timezone.now() - datetime.timedelta(days=10)).date(), }, context={"sponsor": sponsor_foo}, ) - with pytest.raises( - ValidationError, - match=re.escape( - "{'end_date': [ErrorDetail(string='End date cannot be in the past', code='invalid')]}" - ), - ): - ser.is_valid(raise_exception=True) + assert ser.is_valid(raise_exception=True) @pytest.mark.django_db -def test_end_date_expired_role_fail(role, sponsor_foo): - """New end date fail because role has ended""" - # Expire the role to ensure failure +def test_end_date_expired_role_ok(role, sponsor_foo): + """Editing an expired role is allowed""" + # Expire the role role.end_date = datetime.date.today() - datetime.timedelta(days=10) role.save() # Try to change it @@ -86,14 +74,7 @@ def test_end_date_expired_role_fail(role, sponsor_foo): }, context={"sponsor": sponsor_foo}, ) - # Verify that a validation error is raised - with pytest.raises( - ValidationError, - match=re.escape( - "{'end_date': [ErrorDetail(string='Role has ended, cannot change end date', code='invalid')]}" - ), - ): - ser.is_valid(raise_exception=True) + assert ser.is_valid(raise_exception=True) @pytest.mark.django_db -- GitLab