PT-1946: Update person retention period deletion logic

occurrences/tests/test_commands.py

@@ -129,34 +129,13 @@ class TestDeleteRetentionPeriodExceedingContactInfoCommand:
     @pytest.mark.parametrize(
         "flags",
         [
-            pytest.param(
-                {
-                    "delete_event_contact_info": False,
-                    "delete_enrollee_personal_data": False,
-                },
-                id="no_flags",
-            ),
-            pytest.param(
-                {
-                    "delete_event_contact_info": True,
-                    "delete_enrollee_personal_data": True,
-                },
-                id="both_flags",
-            ),
             pytest.param(
                 {
                     "delete_event_contact_info": True,
                     "delete_enrollee_personal_data": False,
                 },
                 id="only_event_contact_info_flag",
             ),
-            pytest.param(
-                {
-                    "delete_event_contact_info": False,
-                    "delete_enrollee_personal_data": True,
-                },
-                id="only_enrollee_personal_data_flag",
-            ),
         ],
     )
     @pytest.mark.parametrize("dry_run", [True, False], ids=["dry_run", "no_dry_run"])

organisations/models.py

@@ -8,7 +8,7 @@
 from django.contrib.auth.models import UserManager
 from django.contrib.postgres.fields import ArrayField
 from django.db import models, router, transaction
-from django.db.models import Max, Q
+from django.db.models import Q
 from django.db.models.deletion import Collector
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
@@ -37,27 +37,34 @@ def user_can_view(self, user):
             return self.none()
 
     def retention_period_exceeded(self):
+        """
+        Return queryset of Person objects whose personal data
+        retention period has exceeded.
+
+        This excludes:
+        - Persons with a user account
+        - Persons with any valid (recent) enrolments
+        """
         earliest_valid_timestamp = timezone.now() - relativedelta(
             months=settings.PERSONAL_DATA_RETENTION_PERIOD_MONTHS
         )
 
-        all_enrolments_too_old = Q(max_enrolment_end_time=None) | Q(
-            max_enrolment_end_time__lt=earliest_valid_timestamp
-        )
-        all_study_groups_too_old = Q(max_studygroup_end_time=None) | Q(
-            max_studygroup_end_time__lt=earliest_valid_timestamp
-        )
-
+        # Return persons who:
+        # 1. Don't have a user account
+        # 2. Don't have any valid enrolments (either as person or study group contact)
+        # 3. Don't have any valid event queue enrolments
         return (
-            # At least for now returns only unauthenticated Person's
-            self.filter(user=None)
-            .annotate(
-                max_enrolment_end_time=Max("enrolment__occurrence__end_time"),
-                max_studygroup_end_time=Max(
-                    "studygroup__enrolments__occurrence__end_time"
-                ),
+            self.exclude(user__isnull=False)
+            .exclude(enrolment__occurrence__end_time__gt=earliest_valid_timestamp)
+            .exclude(
+                studygroup__enrolments__occurrence__end_time__gt=earliest_valid_timestamp
+            )
+            .exclude(
+                studygroup__queued_enrolments__p_event__occurrences__end_time__gt=earliest_valid_timestamp
+            )
+            .exclude(
+                eventqueueenrolment__p_event__occurrences__end_time__gt=earliest_valid_timestamp
             )
-            .filter(all_enrolments_too_old & all_study_groups_too_old)
         )
 
     @transaction.atomic

organisations/tests/test_models.py

@@ -4,7 +4,12 @@
 from django.db import IntegrityError
 from django.utils import timezone
 
-from occurrences.factories import EnrolmentFactory, PalvelutarjotinEventFactory
+from occurrences.factories import (
+    EnrolmentFactory,
+    EventQueueEnrolmentFactory,
+    OccurrenceFactory,
+    PalvelutarjotinEventFactory,
+)
 from organisations.factories import (
     OrganisationFactory,
     OrganisationProposalFactory,
@@ -210,6 +215,84 @@ def test_too_old_personal_data(mock_get_event_data):
     assert all(person in results for person in expected_people)
 
 
+@pytest.mark.django_db
+def test_old_personal_data_deletion_with_valid_enrolments(mock_get_event_data):
+    too_old = timezone.now() - relativedelta(months=24, days=1)
+    still_valid = timezone.now() - relativedelta(days=1)
+
+    (
+        enrolment_person,
+        study_group_person,
+        enrolment_and_study_group_person,
+        no_enrolment_or_study_group_person,
+        event_queue_enrolment_person,
+        event_queue_study_group_person,
+    ) = PersonFactory.create_batch(6, user=None)
+
+    # too old enrolment for the enrolment person
+    EnrolmentFactory(person=enrolment_person, occurrence__end_time=too_old)
+
+    # too old enrolment for the study group person
+    EnrolmentFactory(
+        study_group__person=study_group_person, occurrence__end_time=too_old
+    )
+
+    # too old enrolment for the enrolment and study group person
+    EnrolmentFactory(
+        person=enrolment_and_study_group_person,
+        study_group__person=enrolment_and_study_group_person,
+        occurrence__end_time=too_old,
+    )
+
+    too_old_p_event = PalvelutarjotinEventFactory()
+    OccurrenceFactory(end_time=too_old, p_event=too_old_p_event)
+
+    # too old event queue enrolment for the event queue enrolment person
+    EventQueueEnrolmentFactory(
+        person=event_queue_enrolment_person, p_event=too_old_p_event
+    )
+    # too old event queue enrolment for the event queue study group person
+    EventQueueEnrolmentFactory(
+        study_group__person=event_queue_study_group_person,
+        p_event=too_old_p_event,
+    )
+
+    # valid enrolment for the enrolment person
+    EnrolmentFactory(person=enrolment_person, occurrence__end_time=still_valid)
+    # valid enrolment for the study group person
+    EnrolmentFactory(
+        study_group__person=study_group_person, occurrence__end_time=still_valid
+    )
+
+    valid_p_event = PalvelutarjotinEventFactory()
+    OccurrenceFactory(end_time=still_valid, p_event=valid_p_event)
+
+    # valid event queue enrolment for the event queue enrolment person
+    EventQueueEnrolmentFactory(
+        person=event_queue_enrolment_person, p_event=valid_p_event
+    )
+    # valid enrolment for the event queue enrolment person
+    EventQueueEnrolmentFactory(
+        person=event_queue_enrolment_person, p_event=valid_p_event
+    )
+    # valid enrolment for the event queue study group person
+    EventQueueEnrolmentFactory(
+        study_group__person=event_queue_study_group_person, p_event=valid_p_event
+    )
+    # person with a user
+    PersonFactory(user=UserFactory())
+
+    expected_people = [
+        enrolment_and_study_group_person,
+        no_enrolment_or_study_group_person,
+    ]
+
+    # only persons without any valid enrolments should be returned
+    results = Person.objects.retention_period_exceeded()
+    assert len(results) == len(expected_people)
+    assert all(person in results for person in expected_people)
+
+
 @pytest.mark.django_db
 @pytest.mark.parametrize("delete_via_queryset", (False, True))
 def test_user_deletion_deletes_palvelutarjotin_event_contact_info(delete_via_queryset):