Merge branch 'GREG-404-less-mail-to-guests' into 'master'

Reduce number of emails to guests

See merge request !445

greg/tasks/notify_role_end.py

@@ -2,14 +2,49 @@ import datetime
 import logging
 from collections import defaultdict
 
+from cerebrum_client import CerebrumClient
 from django.conf import settings
 
-from greg.models import Role, Sponsor
+from greg.models import Person, Role, Sponsor
+from greg.utils import get_cerebrum_person_by_greg_id
 from gregui.mailutils.role_ending import RolesEnding
 
 logger = logging.getLogger(__name__)
 
 
+def person_still_has_affiliation(person: Person):
+    client = CerebrumClient(**settings.CEREBRUM_CLIENT)  # type: ignore[arg-type]
+    source = settings.GREG_SOURCE
+    id_type = settings.CEREBRUM_GREG_ID_TYPE
+    cerebrum_person = get_cerebrum_person_by_greg_id(
+        person, client=client, source=source, id_type=id_type
+    )
+    if cerebrum_person is None:
+        return False
+    try:
+        affiliation = client.get_person_affiliations(cerebrum_person.person_id)
+    except ConnectionError as e:
+        logger.exception(
+            "Failed to get affiliation for person_id=%s. Got error=%s",
+            person.pk,
+            e,
+        )
+        return False
+    # Check if person has ansatt or student aff
+    if affiliation is None:
+        return False
+    for aff in affiliation:
+        if aff.affiliation in settings.OTHER_AFFILIATIONS:
+            return True
+    # Checks if person has other guest roles
+    person_roles = person.roles.all()
+    cut_off = datetime.date.today() + datetime.timedelta(days=settings.CUT_OFF)
+    for role in person_roles:
+        if role.end_date > cut_off:
+            return True
+    return False
+
+
 def notify_sponsors_roles_ending() -> list[str]:  # pylint: disable=too-many-locals
     """
     This task notifies sponsors of roles (that are accessible to them)
@@ -118,6 +153,8 @@ def notify_guests_roles_ending() -> list[str]:
     for role in all_roles:
         role_end_notified = False
         person = role.person
+        if person_still_has_affiliation(person):
+            continue
         logger.info(
             "Queueing role end notification for %s",
             person,

greg/tasks/usernames.py

@@ -6,6 +6,7 @@ from django.db.utils import IntegrityError
 from django.utils import timezone
 
 from greg.models import Identity, Person
+from greg.utils import get_cerebrum_person_by_greg_id
 
 logger = logging.getLogger(__name__)
 
@@ -14,8 +15,9 @@ class UsernameImporter:
     def __init__(self):
         self.client = CerebrumClient(**settings.CEREBRUM_CLIENT)
 
-        self.source = "GREG"
-        self.cerebrum_id_type = "gregPersonId"
+        self.source = settings.GREG_SOURCE
+        self.cerebrum_source = settings.CEREBRUM_SOURCE
+        self.cerebrum_id_type = settings.CEREBRUM_GREG_ID_TYPE
         self.id_type = Identity.IdentityType.FEIDE_ID
 
     def get_username(self, person):
@@ -25,7 +27,7 @@ class UsernameImporter:
 
     def update_or_create_person_feide_id(self, person, username):
         existing_feide_id = person.identities.filter(
-            type=self.id_type, source="cerebrum"
+            type=self.id_type, source=self.cerebrum_source
         )
         feide_id = f"{username}@{settings.INSTANCE_NAME}.no"
         if not existing_feide_id:
@@ -34,7 +36,7 @@ class UsernameImporter:
                 Identity.objects.create(
                     person=person,
                     type=self.id_type,
-                    source="cerebrum",
+                    source=self.cerebrum_source,
                     value=feide_id,
                     verified=Identity.Verified.AUTOMATIC,
                     verified_at=timezone.now(),
@@ -63,12 +65,12 @@ class UsernameImporter:
         if version == "with_usernames":
             all_persons = Person.objects.filter(
                 identities__type=Identity.IdentityType.FEIDE_ID,
-                identities__source="cerebrum",
+                identities__source=self.cerebrum_source,
             )
         elif version == "without_usernames":
             all_persons = Person.objects.exclude(
                 identities__type=Identity.IdentityType.FEIDE_ID,
-                identities__source="cerebrum",
+                identities__source=self.cerebrum_source,
             ).filter(registration_completed_date__isnull=False)
         else:
             all_persons = Person.objects.filter(
@@ -80,41 +82,14 @@ class UsernameImporter:
                 "Looking up username for person_id=%s from cerebrum",
                 person.pk,
             )
+            cerebrum_person = get_cerebrum_person_by_greg_id(
+                person=person,
+                client=self.client,
+                id_type=self.cerebrum_id_type,
+                source=self.source,
+            )
             try:
-                persons = list(
-                    self.client.search_person_external_ids(
-                        source_system=self.source,
-                        id_type=self.cerebrum_id_type,
-                        external_id=person.pk,
-                    )
-                )
-            except ConnectionError as e:  # noqa: W0703
-                logger.exception(
-                    "Failed to get person with person_id=%s. Got error=%s. Skipping person",
-                    person.pk,
-                    e,
-                )
-                continue
-            if not persons:
-                logger.info(
-                    "Found no person in cerebrum matching id_type=%s and person_id=%s from source=%s. Skipping person",
-                    self.cerebrum_id_type,
-                    person.pk,
-                    self.source,
-                )
-                continue
-            if len(persons) > 1:
-                logger.info(
-                    "Found %s persons with same id_type=%s and person_id=%s in source_system=%s. Skipping person",
-                    len(persons),
-                    self.cerebrum_id_type,
-                    person.pk,
-                    self.source,
-                )
-                continue
-
-            try:
-                username = self.get_username(persons[0])
+                username = self.get_username(cerebrum_person)
             except ConnectionError as e:
                 logger.exception(
                     "Failed to get username for person_id=%s. Got error=%s. Skipping person",

greg/tests/test_tasks.py

@@ -24,6 +24,126 @@ from greg.tasks import (
 from gregui.models import EmailTemplate
 
 
+@pytest.fixture
+def search_response():
+    return {
+        "external_ids": [
+            {
+                "person_id": 1,
+                "source_system": "GREG",
+                "external_id": 1,
+                "id_type": "gregPersonId",
+            }
+        ]
+    }
+
+
+@pytest.fixture
+def person_account_response():
+    return {
+        "accounts": [
+            {
+                "primary": True,
+                "id": 1,
+                "name": "foo",
+            }
+        ]
+    }
+
+
+@pytest.fixture
+def search_response2():
+    return {
+        "external_ids": [
+            {
+                "person_id": 2,
+                "source_system": "GREG",
+                "external_id": 2,
+                "id_type": "gregPersonId",
+            }
+        ]
+    }
+
+
+@pytest.fixture
+def person_account_response2():
+    return {
+        "accounts": [
+            {
+                "primary": True,
+                "id": 2,
+                "name": "bar",
+            }
+        ]
+    }
+
+
+@pytest.fixture
+def search_response3():
+    return {
+        "external_ids": [
+            {
+                "person_id": 3,
+                "source_system": "GREG",
+                "external_id": 3,
+                "id_type": "gregPersonId",
+            }
+        ]
+    }
+
+
+@pytest.fixture
+def person_account_response3():
+    return {
+        "accounts": [
+            {
+                "primary": True,
+                "id": 3,
+                "name": "foobar",
+            }
+        ]
+    }
+
+
+@pytest.fixture
+def person_affiliations_response():
+    return {
+        "affiliations": [
+            {
+                "status": "active",
+                "create_date": "2024-01-02T11:07:39.915Z",
+                "source_system": "",
+                "affiliation": "131",
+                "last_date": "2024-01-02T11:07:39.915Z",
+                "ou": {"href": "", "id": 185},
+            }
+        ]
+    }
+
+
+@pytest.fixture
+def person_no_affiliations_response():
+    return {"affiliations": []}
+
+
+@pytest.fixture
+def new_role_person(
+    person_foo: Person,
+    role_type_foo: RoleType,
+    sponsor_guy: Sponsor,
+    unit_foo: OrganizationalUnit,
+) -> Role:
+    role = Role.objects.create(
+        person=person_foo,
+        type=role_type_foo,
+        start_date=datetime.date.today(),
+        end_date=datetime.date.today() + datetime.timedelta(days=90),
+        sponsor=sponsor_guy,
+        orgunit=unit_foo,
+    )
+    return Role.objects.get(id=role.id)
+
+
 @pytest.fixture
 def role_end_reminder_sponsor_template():
     et = EmailTemplate.objects.create(
@@ -163,18 +283,65 @@ def test_only_close_sponsors_get_email(
 @pytest.mark.django_db
 @override_settings(GUEST_NOTIFIER_FIRST=10, GUEST_NOTIFIER_SECOND=5)
 def test_notify_guest_roles_ending(
+    requests_mock,
     role_end_reminder_guest_template,
     role_person_foo2,
     role_person_bar2,
     role_person_bar,
+    search_response,
+    search_response2,
+    search_response3,
+    person_account_response,
+    person_account_response2,
+    person_account_response3,
+    person_no_affiliations_response,
     caplog,
 ):
-    mail.outbox = []
+    requests_mock.get(
+        "https://example.com/cerebrum/v1/search/persons/external-ids?source_system=GREG&id_type=gregPersonId&external_id=1",
+        json=search_response,
+    )
+    requests_mock.get(
+        "https://example.com/cerebrum/v1/persons/1/accounts",
+        json=person_account_response,
+    )
+    requests_mock.get(
+        "https://example.com/cerebrum/v1/persons/1/affiliations",
+        json=person_no_affiliations_response,
+    )
+    requests_mock.get(
+        "https://example.com/cerebrum/v1/search/persons/external-ids?source_system=GREG&id_type=gregPersonId&external_id=2",
+        json=search_response2,
+    )
+    requests_mock.get(
+        "https://example.com/cerebrum/v1/persons/2/accounts",
+        json=person_account_response2,
+    )
+    requests_mock.get(
+        "https://example.com/cerebrum/v1/persons/2/affiliations",
+        json=person_no_affiliations_response,
+    )
+    requests_mock.get(
+        "https://example.com/cerebrum/v1/search/persons/external-ids?source_system=GREG&id_type=gregPersonId&external_id=3",
+        json=search_response3,
+    )
+    requests_mock.get(
+        "https://example.com/cerebrum/v1/persons/3/accounts",
+        json=person_account_response3,
+    )
+    requests_mock.get(
+        "https://example.com/cerebrum/v1/persons/3/affiliations",
+        json=person_no_affiliations_response,
+    )
     task_ids = notify_role_end.notify_guests_roles_ending()
     assert len(task_ids) == 2
     assert "Queueing a role end notification email to bar@example.org" in caplog.text
     assert f"Your guest role {role_person_bar2.type.name_en} at UiO" in caplog.text
     assert f"Your guest role {role_person_bar.type.name_en} at UiO" in caplog.text
+    assert (
+        "Queueing a role end notification email to foo@example.org" not in caplog.text
+    )
+    assert f"Your guest role {role_person_foo2.type.name_en} at UiO" not in caplog.text
     role_person_foo2.end_date = datetime.date.today() + datetime.timedelta(days=5)
     role_person_foo2.save()
     task_ids = notify_role_end.notify_guests_roles_ending()
@@ -183,6 +350,64 @@ def test_notify_guest_roles_ending(
     assert f"Your guest role {role_person_foo2.type.name_en} at UiO" in caplog.text
 
 
+@pytest.mark.django_db
+@override_settings(GUEST_NOTIFIER_FIRST=10, GUEST_NOTIFIER_SECOND=4)
+def test_not_notify_guest_turned_ansatt(
+    requests_mock,
+    search_response,
+    person_account_response,
+    person_affiliations_response,
+    role_person_foo2,
+    caplog,
+):
+    requests_mock.get(
+        "https://example.com/cerebrum/v1/search/persons/external-ids?source_system=GREG&id_type=gregPersonId&external_id=1",
+        json=search_response,
+    )
+    requests_mock.get(
+        "https://example.com/cerebrum/v1/persons/1/accounts",
+        json=person_account_response,
+    )
+    requests_mock.get(
+        "https://example.com/cerebrum/v1/persons/1/affiliations",
+        json=person_affiliations_response,
+    )
+
+    task_ids = notify_role_end.notify_guests_roles_ending()
+    assert len(task_ids) == 0
+    assert "Found 1 roles that are ending in 4 days" in caplog.text
+    assert "Queued 0 tasks" in caplog.text
+
+
+@pytest.mark.django_db
+def test_not_notify_guest_with_new_guest_role(
+    requests_mock,
+    search_response,
+    person_account_response,
+    person_no_affiliations_response,
+    role_person_foo2,
+    new_role_person,
+    caplog,
+):
+    requests_mock.get(
+        "https://example.com/cerebrum/v1/search/persons/external-ids?source_system=GREG&id_type=gregPersonId&external_id=1",
+        json=search_response,
+    )
+    requests_mock.get(
+        "https://example.com/cerebrum/v1/persons/1/accounts",
+        json=person_account_response,
+    )
+    requests_mock.get(
+        "https://example.com/cerebrum/v1/persons/1/affiliations",
+        json=person_no_affiliations_response,
+    )
+
+    task_ids = notify_role_end.notify_guests_roles_ending()
+    assert len(task_ids) == 0
+    assert "Found 1 roles that are ending in 4 days" in caplog.text
+    assert "Queued 0 tasks" in caplog.text
+
+
 @pytest.fixture
 def old_unit():
     ou = OrganizationalUnit.objects.create(name_nb="a", name_en="b")

greg/utils.py

+import logging
 import re
 import typing
 from datetime import date, datetime, timedelta, timezone as dt_timezone
@@ -17,6 +18,8 @@ from greg.models import (
 )
 from gregui.mailutils.invite_guest import InviteGuest
 
+logger = logging.getLogger(__name__)
+
 
 def camel_to_snake(s: str) -> str:
     """Turns `FooBar` into `foo_bar`."""
@@ -247,3 +250,40 @@ def string_contains_illegal_chars(string: str) -> bool:
             string,
         )
     )
+
+
+def get_cerebrum_person_by_greg_id(person: Person, client, source: str, id_type: str):
+    try:
+        persons = list(
+            client.search_person_external_ids(
+                source_system=source,
+                id_type=id_type,
+                external_id=person.pk,
+            )
+        )
+    except ConnectionError as e:
+        logger.exception(
+            "Failed to get person with person_id=%s. Got error=%s. Skipping person",
+            person.pk,
+            e,
+        )
+        return None
+    if not persons:
+        logger.info(
+            "Found no person in cerebrum matching id_type=%s and person_id=%s from source=%s",
+            id_type,
+            person.pk,
+            source,
+        )
+        return None
+    if len(persons) > 1:
+        logger.info(
+            "Found %s persons with same id_type=%s and person_id=%s in source_system=%s",
+            len(persons),
+            id_type,
+            person.pk,
+            source,
+        )
+        return None
+
+    return persons[0]

gregsite/settings/base.py

@@ -295,6 +295,7 @@ INTERNAL_RK_PREFIX = "no.{instance}.greg".format(instance=INSTANCE_NAME)
 BASE_URL = "https://example.org"
 
 FEIDE_SOURCE = "feide"
+CEREBRUM_SOURCE = "cerebrum"
 
 # The default duration for a new invitation link, in days
 INVITATION_DURATION = 30
@@ -392,3 +393,13 @@ ROLE_EXPIRY_DAYS = 1826  # 5 years = 1826.21 days
 
 # Default schedule for the task deleting expired roles.
 ROLE_REMOVAL_SCHEDULE_TYPE = "W"
+
+# Ansatt and student affiliation codes in cerebrum
+OTHER_AFFILIATIONS = ["131", "129"]
+
+# Lenght of other active guest role to not notify of expiring role
+CUT_OFF = 60  # Two months
+
+# Sources used to get greg person from cerebrum
+GREG_SOURCE = "GREG"
+CEREBRUM_GREG_ID_TYPE = "gregPersonId"

gregui/authentication/auth_backends.py

@@ -389,7 +389,7 @@ class GregOIDCBackend(ValidatingOIDCBackend):
         feide_ids: list[Identity] = [
             x
             for x in person.identities.filter(type=Identity.IdentityType.FEIDE_ID)
-            .exclude(source="cerebrum")
+            .exclude(source=settings.CEREBRUM_SOURCE)
             .all()
             if x.value.split("@")[-1] == feide_inst
         ]