From b055277a0260b79d58ff0697fcac2ed795d36336 Mon Sep 17 00:00:00 2001
From: Andreas Ellewsen <andreas@ellewsen.no>
Date: Thu, 9 Jun 2022 11:16:00 +0200
Subject: [PATCH 1/2] Move end role notifications to day after end

By moving the notifications to midnight on the day they expire we
simplify the work of systems integrating with greg. The frontend has
also been updated to say that end dates are "to (including)" to clarify
that the roles are valid on their last day.

Resolves: GREG-2688
---
 frontend/public/locales/en/common.json |  2 +-
 frontend/public/locales/nb/common.json |  2 +-
 frontend/public/locales/nn/common.json |  2 +-
 greg/signals.py                        | 89 +++++++++++++++++---------
 greg/tests/test_signals.py             | 44 ++++++++++---
 greg/utils.py                          | 12 ++--
 6 files changed, 102 insertions(+), 49 deletions(-)

diff --git a/frontend/public/locales/en/common.json b/frontend/public/locales/en/common.json
index 50e97ac8..b9bde6e8 100644
--- a/frontend/public/locales/en/common.json
+++ b/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",
diff --git a/frontend/public/locales/nb/common.json b/frontend/public/locales/nb/common.json
index a6849c3e..e107a8bb 100644
--- a/frontend/public/locales/nb/common.json
+++ b/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",
diff --git a/frontend/public/locales/nn/common.json b/frontend/public/locales/nn/common.json
index aa5b64c7..b689424e 100644
--- a/frontend/public/locales/nn/common.json
+++ b/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",
diff --git a/greg/signals.py b/greg/signals.py
index de65ab3e..cedcf0b1 100644
--- a/greg/signals.py
+++ b/greg/signals.py
@@ -1,6 +1,6 @@
 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(
diff --git a/greg/tests/test_signals.py b/greg/tests/test_signals.py
index 31e4da47..b7b4731c 100644
--- a/greg/tests/test_signals.py
+++ b/greg/tests/test_signals.py
@@ -1,6 +1,8 @@
 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
diff --git a/greg/utils.py b/greg/utils.py
index b8d7af1d..76b2f583 100644
--- a/greg/utils.py
+++ b/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:
+def date_to_datetime_midnight(in_date: date) -> 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
+    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():
-- 
GitLab


From 37b8b6cf0f861ef301d4cb924c604b6fb69d3ba5 Mon Sep 17 00:00:00 2001
From: elg <elg@uio.no>
Date: Mon, 13 Jun 2022 09:55:39 +0200
Subject: [PATCH 2/2] Update doc string to match function

---
 greg/utils.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/greg/utils.py b/greg/utils.py
index 76b2f583..1afb6f20 100644
--- a/greg/utils.py
+++ b/greg/utils.py
@@ -106,7 +106,7 @@ def _compute_checksum(input_digits: str) -> bool:
 
 
 def date_to_datetime_midnight(in_date: date) -> datetime:
-    """Convert a date or str object to a datetime object with timezone utc"""
+    """Convert a date object to a datetime object with timezone utc"""
     return datetime.combine(in_date, datetime.min.time(), tzinfo=timezone.utc)
 
 
-- 
GitLab