Merge branch 'GREG-268-notify-endrole-later' into 'master'

Move end role notifications to day after end

See merge request !328

frontend/public/locales/en/common.json

@@ -14,7 +14,7 @@
     "nationalIdNumber": "National ID number",
     "roleType": "Role",
     "roleStartDate": "From",
-    "roleEndDate": "To",
+    "roleEndDate": "To (including)",
     "comment": "Comment",
     "searchable": "Available in search?",
     "email": "E-mail",

frontend/public/locales/nb/common.json

@@ -14,7 +14,7 @@
     "nationalIdNumber": "Fødselsnummer/D-nummer",
     "roleType": "Gjesterolle",
     "roleStartDate": "Fra",
-    "roleEndDate": "Til",
+    "roleEndDate": "Til og med",
     "comment": "Kommentar",
     "searchable": "Synlig i søk?",
     "email": "E-post",

frontend/public/locales/nn/common.json

@@ -14,7 +14,7 @@
     "nationalIdNumber": "Fødselsnummer/D-nummer",
     "roleType": "Gjesterolle",
     "roleStartDate": "Frå",
-    "roleEndDate": "Til",
+    "roleEndDate": "Til og med",
     "comment": "Kommentar",
     "searchable": "Synleg i søk?",
     "email": "E-post",

greg/signals.py

 import datetime
 import time
-from typing import Dict, Union
+from typing import Callable, Dict, Union
 
 from django_structlog.signals import bind_extra_request_metadata
 from django.db import models
@@ -15,7 +15,7 @@ from greg.models import (
     Consent,
     ConsentType,
 )
-from greg.utils import date_to_datetime_midnight
+from greg.utils import date_to_datetime_midnight, str2date
 
 
 @receiver(bind_extra_request_metadata)
@@ -81,16 +81,18 @@ def _store_notification(identifier, object_type, operation, **kwargs):
     )
 
 
-def _queue_role_notification(role_id: int, created: str, attrname: str):
-    """Create a notification if the date in attrname of role is today"""
+def _queue_role_notification(
+    role_id: int,
+    created: str,
+    should_notify: Callable[[Role], bool],
+):
+    """Create a notification if provided function says so"""
     try:
         instance = Role.objects.get(id=role_id)
     except Role.DoesNotExist:
         # Role was deleted, ignore it
         return
-    if getattr(instance, attrname) != datetime.date.today():
-        # Date was changed after this task was queued, ignore it, the new one will take
-        # care of it
+    if not should_notify(instance):
         return
     meta = _create_metadata(instance)
     operation = "add" if created else "update"
@@ -104,12 +106,20 @@ def _queue_role_notification(role_id: int, created: str, attrname: str):
 
 def _queue_role_start_notification(role_id: int, created: str):
     """Create a notification for the role if start_date is today"""
-    _queue_role_notification(role_id, created, "start_date")
+
+    def should_notify(instance):
+        return instance.start_date == datetime.date.today()
+
+    _queue_role_notification(role_id, created, should_notify)
 
 
 def _queue_role_end_notification(role_id: int, created: str):
-    """Create a notification for the role if end_date is today"""
-    _queue_role_notification(role_id, created, "end_date")
+    """Create a notification for the role if end_date was yesterday"""
+
+    def should_notify(instance: Role):
+        return instance.end_date == datetime.date.today() - datetime.timedelta(days=1)
+
+    _queue_role_notification(role_id, created, should_notify)
 
 
 @receiver(models.signals.pre_save, dispatch_uid="add_changed_fields_callback")
@@ -128,34 +138,49 @@ def add_changed_fields_callback(sender, instance, raw, *args, **kwargs):
     )
 
 
+def _handle_role_save_signal(instance: Role):
+    """
+    Side effects of saving a Role
+
+    Schedules the production of a Notification on the start date and on
+    the day following the end date of the Role if their dates are in
+    the future
+    """
+    today = datetime.date.today()
+    if (
+        "start_date" in instance._changed_fields  # pylint: disable=protected-access
+        and instance.start_date
+        and str2date(instance.start_date) > today
+    ):
+        Schedule.objects.create(
+            func="greg.signals._queue_role_start_notification",
+            args=f"{instance.id},True",
+            next_run=date_to_datetime_midnight(str2date(instance.start_date)),
+            schedule_type=Schedule.ONCE,
+        )
+    if (
+        "end_date" in instance._changed_fields  # pylint: disable=protected-access
+        and str2date(instance.end_date) > today
+    ):
+        Schedule.objects.create(
+            func="greg.signals._queue_role_end_notification",
+            args=f"{instance.id},True",
+            next_run=date_to_datetime_midnight(
+                str2date(instance.end_date) + datetime.timedelta(days=1)
+            ),
+            schedule_type=Schedule.ONCE,
+        )
+
+
 @receiver(models.signals.post_save, dispatch_uid="save_notification_callback")
 def save_notification_callback(sender, instance, created, *args, **kwargs):
+    """Do things when supported models are saved"""
     if not isinstance(instance, SUPPORTED_MODELS):
         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",
-                args=f"{instance.id},True",
-                next_run=date_to_datetime_midnight(instance.start_date),
-                schedule_type=Schedule.ONCE,
-            )
-        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",
-                next_run=date_to_datetime_midnight(instance.end_date),
-                schedule_type=Schedule.ONCE,
-            )
+        _handle_role_save_signal(instance)
+
     meta = _create_metadata(instance)
     operation = "add" if created else "update"
     _store_notification(

greg/tests/test_signals.py

 import datetime
 import pytest
 
+from django_q.models import Schedule
+
 from greg.models import Notification, Role
 from greg.signals import _queue_role_start_notification, _queue_role_end_notification
 
@@ -45,20 +47,46 @@ def test_queue_role_start_notification(role_today):
 
 @pytest.mark.django_db
 def test_queue_role_end_notification(role_today):
-    """Check that a notification is produced if the role ends today"""
-    assert Notification.objects.all().count() == 3
-    _queue_role_end_notification(role_today.id, True)
+    """Check that a notification is produced if the role ends yesterday"""
+    role_today.end_date = datetime.date.today() - datetime.timedelta(days=1)
+    role_today.save()
     assert Notification.objects.all().count() == 4
+    _queue_role_end_notification(role_today.id, True)
+    assert Notification.objects.all().count() == 5
 
 
 @pytest.mark.django_db
-def test_queue_role_end_notification_wrong_date(role_today):
-    """Check that a notification is produced if the role ends today"""
-    role_today.end_date = datetime.date.today() - datetime.timedelta(days=2)
+def test_role_save_schedules_notifications(role_today):
+    """Verify that future end or start dates schedule future notifications"""
+    oneday = datetime.timedelta(days=1)
+    today = datetime.date.today()
+    tomorrow = today + oneday
+    yesterday = today - oneday
+    assert Notification.objects.all().count() == 3
+    assert Schedule.objects.all().count() == 0
+
+    # Future end date schedules
+    role_today.end_date = tomorrow
     role_today.save()
-    assert Notification.objects.all().count() == 4
+    assert Schedule.objects.all().count() == 1
+    # Future start date schedules
+    role_today.start_date = tomorrow
+    role_today.save()
+    assert Schedule.objects.all().count() == 2
+    # Past end date does not schedule (should be impossible in the application but test anyway)
+    role_today.end_date = yesterday
+    role_today.save()
+    assert Schedule.objects.all().count() == 2  # Past start date schedules
+    role_today.start_date = yesterday
+    role_today.save()
+
+
+@pytest.mark.django_db
+def test_queue_role_end_notification_wrong_date(role_today):
+    """Check that a notification is not produced if the role does not end yesterday"""
+    assert Notification.objects.all().count() == 3
     _queue_role_end_notification(role_today.id, True)
-    assert Notification.objects.all().count() == 4
+    assert Notification.objects.all().count() == 3
 
 
 @pytest.mark.django_db

greg/utils.py

@@ -105,13 +105,13 @@ def _compute_checksum(input_digits: str) -> bool:
     return k1 < 10 and k2 < 10 and k1 == d[9] and k2 == d[10]
 
 
-def date_to_datetime_midnight(in_date: typing.Union[date, str]) -> datetime:
-    """Convert a date or str object to a datetime object with timezone utc"""
-    start_date = (
-        datetime.strptime(in_date, "%Y-%M-%d") if isinstance(in_date, str) else in_date
-    )
-    start = datetime.combine(start_date, datetime.min.time(), tzinfo=timezone.utc)
-    return start
+def date_to_datetime_midnight(in_date: date) -> datetime:
+    """Convert a date object to a datetime object with timezone utc"""
+    return datetime.combine(in_date, datetime.min.time(), tzinfo=timezone.utc)
+
+
+def str2date(in_date: typing.Union[str, date]):
+    return date.fromisoformat(in_date) if isinstance(in_date, str) else in_date
 
 
 def get_default_invitation_expire_date_from_now():