diff --git a/greg/admin.py b/greg/admin.py index 809f5bfb29bf7be379a11a2aaade54fa0643e1ec..c19389dce61a8aa022051be0338ce6a6f0e0d6ca 100644 --- a/greg/admin.py +++ b/greg/admin.py @@ -5,6 +5,7 @@ from greg.models import ( OuIdentifier, Invitation, InvitationLink, + Notification, Person, Role, RoleType, @@ -133,6 +134,18 @@ class InvitationLinkAdmin(VersionAdmin): readonly_fields = ("uuid",) +class NotificationAdmin(VersionAdmin): + list_display = ( + "id", + "object_type", + "identifier", + "operation", + "created", + "updated", + "issued_at", + ) + + admin.site.register(Person, PersonAdmin) admin.site.register(Role, RoleAdmin) admin.site.register(RoleType, RoleTypeAdmin) @@ -145,3 +158,4 @@ admin.site.register(SponsorOrganizationalUnit, SponsorOrganizationalUnitAdmin) admin.site.register(Invitation, InvitationAdmin) admin.site.register(InvitationLink, InvitationLinkAdmin) admin.site.register(OuIdentifier, IdentifierAdmin) +admin.site.register(Notification, NotificationAdmin) diff --git a/greg/management/commands/task_scheduler.py b/greg/management/commands/task_scheduler.py deleted file mode 100644 index 29fe967ebe2d45d04ee357af746eb3a282467a63..0000000000000000000000000000000000000000 --- a/greg/management/commands/task_scheduler.py +++ /dev/null @@ -1,36 +0,0 @@ -import logging -import logging.config -import time - -from django.conf import settings -from django.core.management.base import BaseCommand - -from greg.schedule import ExpiringRolesNotification - -logging.config.dictConfig(settings.LOGGING) -logger = logging.getLogger(__name__) - - -class Command(BaseCommand): - """ - This command starts a basic task runner. All tasks it is supposed to - run are given explicitly in code - """ - - help = "Start schedule task runner" - - def handle(self, *args, **options): - - logger.info("Task scheduler started") - - # For now there is just one task, but the idea is that more can - # be added - expiring_role_notification_task = ExpiringRolesNotification() - - while True: - expiring_role_notification_task.run() - # The single task is set up for far only needs to run once - # a day, but running every 6 hours just in case a run fails, - # it is up the task it self to figure out if needs to do - # something every time it is called - time.sleep(60 * 60 * 6) diff --git a/greg/migrations/0013_delete_scheduletask.py b/greg/migrations/0013_delete_scheduletask.py new file mode 100644 index 0000000000000000000000000000000000000000..9c3b85d65f604c9595bd1f6cf63641cfd282784e --- /dev/null +++ b/greg/migrations/0013_delete_scheduletask.py @@ -0,0 +1,16 @@ +# Generated by Django 3.2.8 on 2021-11-04 10:02 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("greg", "0012_ou_identifiers"), + ] + + operations = [ + migrations.DeleteModel( + name="ScheduleTask", + ), + ] diff --git a/greg/models.py b/greg/models.py index 58ed1c2e3ca065b9bc8bc9dbc0aaa19e8366838a..168c69d37616bc611e64926fcbf63c741f20eb73 100644 --- a/greg/models.py +++ b/greg/models.py @@ -477,20 +477,6 @@ class SponsorOrganizationalUnit(BaseModel): ) -class ScheduleTask(models.Model): - """ - Stores information about a task - """ - - name = models.CharField(max_length=32) - last_completed = models.DateTimeField(null=True) - - def __repr__(self): - return "{}(id={!r}, name={!r}, last_completed={!r})".format( - self.__class__.__name__, self.pk, self.name, self.last_completed - ) - - class InvitationLink(BaseModel): """ Link to an invitation. diff --git a/greg/schedule.py b/greg/schedule.py deleted file mode 100644 index ee08c9a79ce2285b1d7189ce9cbfcf7d000713c9..0000000000000000000000000000000000000000 --- a/greg/schedule.py +++ /dev/null @@ -1,89 +0,0 @@ -import time -from datetime import date, datetime, timedelta -from typing import Optional - -from abc import ABC, abstractmethod -from django.utils import timezone - -from greg.models import Role, Notification, ScheduleTask - - -class BaseSchedule(ABC): - """ - Provides common methods for tasks - """ - - task_object: ScheduleTask - - def __init__(self, name: str): - self.task_object = ScheduleTask.objects.get(name=name) - - def run(self): - self._run_internal() - self.task_object.last_completed = timezone.now() - self.task_object.save() - - @abstractmethod - def _run_internal(self): - pass - - def get_last_run(self) -> Optional[datetime]: - self.task_object.refresh_from_db() - return self.task_object.last_completed - - def _store_notification( - self, identifier, object_type, operation, **kwargs - ) -> Notification: - return Notification.objects.create( - identifier=identifier, - object_type=object_type, - operation=operation, - issued_at=int(time.time()), - meta=kwargs, - ) - - -class ExpiringRolesNotification(BaseSchedule): - """ - This task does a simple check for roles that will expire in 30 days - and creates entries in the notification table for them. - - There should be an entry in the ScheduleTask-table with name role_expiration - before this task is run. - - Some assumptions that are made: - - - The task will be run every day, it does - not keep track of which roles it has or has not notified, it only - looks at which roles will expire in exactly 30 days - - - If there are roles created that expire in less than 30 days, no - notification is necessary - """ - - EXPIRATION_THRESHOLD_DAYS = 30 - - def __init__(self): - # Will raise exception if there is not exactly one result - super().__init__("role_expiration") - - def _run_internal(self): - last_run = self.get_last_run() - # Only run once per day - if last_run is None or last_run.date() != date.today(): - check_date = datetime.today() + timedelta( - days=self.EXPIRATION_THRESHOLD_DAYS - ) - self.__get_roles_about_to_expire(check_date) - - def __get_roles_about_to_expire(self, end_date: date): - roles_about_to_expire = Role.objects.filter(end_date=end_date) - - for role in roles_about_to_expire: - meta = {"person_id": role.person.id, "type_id": role.type.id} - self._store_notification( - identifier=role.id, - object_type="PersonRole", - operation="expire_reminder", - **meta - ) diff --git a/greg/signals.py b/greg/signals.py index 3f52d8615325965feb9265fe6f069903195b79ae..5780e614f0735add171167a836264e4982bcb815 100644 --- a/greg/signals.py +++ b/greg/signals.py @@ -1,10 +1,11 @@ +import datetime import time import logging from typing import Dict, Union from django.db import models from django.dispatch import receiver - +from django_q.models import Schedule from greg.models import ( Person, Role, @@ -14,6 +15,7 @@ from greg.models import ( Consent, ConsentType, ) +from greg.utils import date_to_datetime_midnight logger = logging.getLogger(__name__) @@ -74,6 +76,37 @@ 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""" + 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 + return + meta = _create_metadata(instance) + operation = "add" if created else "update" + _store_notification( + identifier=instance.id, + object_type=instance._meta.object_name, + operation=operation, + **meta, + ) + + +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 _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") + + @receiver(models.signals.pre_save, dispatch_uid="add_changed_fields_callback") def add_changed_fields_callback(sender, instance, raw, *args, **kwargs): """ @@ -94,13 +127,32 @@ def add_changed_fields_callback(sender, instance, raw, *args, **kwargs): def save_notification_callback(sender, instance, created, *args, **kwargs): if not isinstance(instance, SUPPORTED_MODELS): return + # Queue future notifications on start and end date for roles + if isinstance(instance, Role): + if ( + "start_date" in instance._changed_fields # pylint: disable=protected-access + and instance.start_date + ): + 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 + 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, + ) meta = _create_metadata(instance) operation = "add" if created else "update" _store_notification( identifier=instance.id, object_type=instance._meta.object_name, operation=operation, - **meta + **meta, ) @@ -113,7 +165,7 @@ def delete_notification_callback(sender, instance, *args, **kwargs): identifier=instance.id, object_type=instance._meta.object_name, operation="delete", - **meta + **meta, ) @@ -146,7 +198,7 @@ def m2m_changed_notification_callback( identifier=pc.id, object_type=Consent._meta.object_name, operation=operation, - **meta + **meta, ) elif sender is Role: roles = [] @@ -161,7 +213,7 @@ def m2m_changed_notification_callback( identifier=pr.id, object_type=Role._meta.object_name, operation=operation, - **meta + **meta, ) diff --git a/greg/tests/models/test_scheduletask.py b/greg/tests/models/test_scheduletask.py deleted file mode 100644 index 5b2e199d6ec0609c88700fbe12d057d6163d0746..0000000000000000000000000000000000000000 --- a/greg/tests/models/test_scheduletask.py +++ /dev/null @@ -1,17 +0,0 @@ -import pytest - -from greg.models import ScheduleTask - - -@pytest.fixture -def scheduletask(): - ScheduleTask.objects.create(name="foo", last_completed="2020-10-15T23:04Z") - return ScheduleTask.objects.get(id=1) - - -@pytest.mark.django_db -def test_scheduletask_repr(scheduletask): - assert ( - repr(scheduletask) - == "ScheduleTask(id=1, name='foo', last_completed=datetime.datetime(2020, 10, 15, 23, 4, tzinfo=<UTC>))" - ) diff --git a/greg/tests/test_expire_role.py b/greg/tests/test_expire_role.py deleted file mode 100644 index 0eda7855840c161104dade3cf64ed9cc71542197..0000000000000000000000000000000000000000 --- a/greg/tests/test_expire_role.py +++ /dev/null @@ -1,68 +0,0 @@ -from datetime import datetime, timedelta - -import pytest -from django.db.models import Q -from django.utils import timezone - -from greg.models import ( - ScheduleTask, - RoleType, - Person, - OrganizationalUnit, - Role, - Notification, - Sponsor, -) -from greg.schedule import ExpiringRolesNotification - - -@pytest.fixture -def role_task(): - ScheduleTask.objects.create( - name="role_expiration", last_completed=timezone.now() - timedelta(days=1) - ) - - -@pytest.fixture -def role( - person: Person, - role_type_bar: RoleType, - unit_foo: OrganizationalUnit, - sponsor_guy: Sponsor, -) -> Role: - return Role.objects.create( - person=person, - type=role_type_bar, - orgunit=unit_foo, - start_date="2020-03-05", - end_date=datetime.today() + timedelta(days=30), - contact_person_unit="Contact Person", - available_in_search=True, - sponsor=sponsor_guy, - ) - - -@pytest.mark.django_db -def test_role_picked_up(role_task: ScheduleTask, role: Role): - role_notification = ExpiringRolesNotification() - assert len(Notification.objects.filter(~Q(operation="add"))) == 0 - role_notification.run() - notification = Notification.objects.get(operation="expire_reminder") - assert notification.identifier == role.id - role_notification.run() - notifications = Notification.objects.filter(operation="expire_reminder") - assert len(notifications) == 1 - - -@pytest.mark.django_db -def test_no_notification_for_role_not_about_to_expire( - role_task: ScheduleTask, role: Role -): - role.end_date = datetime.today() + timedelta(days=31) - role.save() - role_notification = ExpiringRolesNotification() - assert len(Notification.objects.filter(operation="expire_reminder")) == 0 - # Role should not be picked up since it expires in 31 days - role_notification.run() - notifications = Notification.objects.filter(operation="expire_reminder") - assert len(notifications) == 0 diff --git a/greg/tests/test_notifications.py b/greg/tests/test_notifications.py index e5b68d0a6f4829681b3a3f663c2ba198131972cf..c0fd917da896d0866d72b3dc0729bc357c1a5014 100644 --- a/greg/tests/test_notifications.py +++ b/greg/tests/test_notifications.py @@ -15,22 +15,25 @@ from greg.models import ( @pytest.fixture def org_unit_bar() -> OrganizationalUnit: - return OrganizationalUnit.objects.create() + org = OrganizationalUnit.objects.create() + return OrganizationalUnit.objects.get(pk=org.id) @pytest.fixture def sponsor() -> Sponsor: - return Sponsor.objects.create(feide_id="sponsor_id") + sp = Sponsor.objects.create(feide_id="sponsor_id") + return Sponsor.objects.get(pk=sp.id) @pytest.fixture def identity(person: Person) -> Identity: - return Identity.objects.create( + ident = Identity.objects.create( person=person, type=Identity.IdentityType.PASSPORT_NUMBER, source="Test", value="12345678901", ) + return Identity.objects.get(pk=ident.id) @pytest.mark.django_db diff --git a/greg/tests/test_signals.py b/greg/tests/test_signals.py index 1570fd26c785cc228fe2e339c576841a3c16fb39..31e4da470a1f3503e152e3f432ef4ea5bdf27f45 100644 --- a/greg/tests/test_signals.py +++ b/greg/tests/test_signals.py @@ -1,6 +1,22 @@ +import datetime import pytest -from greg.models import Notification +from greg.models import Notification, Role +from greg.signals import _queue_role_start_notification, _queue_role_end_notification + + +@pytest.fixture +def role_today(person, role_type_test_guest, sponsor_guy, unit_foo): + """A test role with end and start date today.""" + role = Role.objects.create( + person=person, + type=role_type_test_guest, + sponsor=sponsor_guy, + orgunit=unit_foo, + start_date=datetime.date.today(), + end_date=datetime.date.today(), + ) + return Role.objects.get(pk=role.id) @pytest.mark.django_db @@ -17,3 +33,37 @@ def test_delete_signal_ou(unit_foo): assert Notification.objects.count() == 0 unit_foo.delete() assert Notification.objects.count() == 0 + + +@pytest.mark.django_db +def test_queue_role_start_notification(role_today): + """Check that a notification is produced if the role starts today""" + assert Notification.objects.all().count() == 3 + _queue_role_start_notification(role_today.id, True) + assert Notification.objects.all().count() == 4 + + +@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) + assert Notification.objects.all().count() == 4 + + +@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) + role_today.save() + assert Notification.objects.all().count() == 4 + _queue_role_end_notification(role_today.id, True) + assert Notification.objects.all().count() == 4 + + +@pytest.mark.django_db +def test_queue_role_end_notification_role_deleted(): + """Check that a notification is not produced if the role was deleted""" + assert Notification.objects.all().count() == 0 + _queue_role_end_notification(10, True) + assert Notification.objects.all().count() == 0 diff --git a/greg/utils.py b/greg/utils.py index 8408087a224f8877f6be4c619434eae081bccdce..a7a27171c3f441ebfebc0201c4fc621e0e76223c 100644 --- a/greg/utils.py +++ b/greg/utils.py @@ -1,5 +1,8 @@ import re -from datetime import date +import typing +from datetime import date, datetime + +from django.utils import timezone def camel_to_snake(s: str) -> str: @@ -90,3 +93,12 @@ def _compute_checksum(input_digits: str) -> bool: k2 = 0 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 diff --git a/mypy.ini b/mypy.ini index 4b3066fc11e7e93a77557f2f7735c495474657ca..9b205574d67da2da925ac0b524dc1b3cd3dd76c0 100644 --- a/mypy.ini +++ b/mypy.ini @@ -20,6 +20,9 @@ ignore_missing_imports = True [mypy-django_extensions.db.fields] ignore_missing_imports = True +[mypy-django_q.*] +ignore_missing_imports = True + [mypy-django_filters.*] ignore_missing_imports = True