feat: Keep track of deleted unverified users (#2174)

* feat: Keep track of deleted unverified users

* Fix test

* Feedback

* Update cron job to anonymise users instead

* Update return type

* Merge branch 'master' into daily_activity_deleted_users

cfl_common/common/migrations/0044_update_activity_models.py

@@ -0,0 +1,33 @@
+# Generated by Django 3.2.20 on 2023-09-14 16:26
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('common', '0043_add_total_activity'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='dailyactivity',
+            name='anonymised_unverified_independents',
+            field=models.PositiveIntegerField(default=0),
+        ),
+        migrations.AddField(
+            model_name='dailyactivity',
+            name='anonymised_unverified_teachers',
+            field=models.PositiveIntegerField(default=0),
+        ),
+        migrations.AddField(
+            model_name='totalactivity',
+            name='anonymised_unverified_independents',
+            field=models.PositiveIntegerField(default=0),
+        ),
+        migrations.AddField(
+            model_name='totalactivity',
+            name='anonymised_unverified_teachers',
+            field=models.PositiveIntegerField(default=0),
+        ),
+    ]

cfl_common/common/models.py

@@ -345,12 +345,14 @@ class DailyActivity(models.Model):
     teacher_lockout_resets = models.PositiveIntegerField(default=0)
     indy_lockout_resets = models.PositiveIntegerField(default=0)
     school_student_lockout_resets = models.PositiveIntegerField(default=0)
+    anonymised_unverified_teachers = models.PositiveIntegerField(default=0)
+    anonymised_unverified_independents = models.PositiveIntegerField(default=0)
 
     class Meta:
         verbose_name_plural = "Daily activities"
 
     def __str__(self):
-        return f"Activity on {self.date}: CSV clicks: {self.csv_click_count}, login cards clicks: {self.login_cards_click_count}, primary pack downloads: {self.primary_coding_club_downloads}, python pack downloads: {self.python_coding_club_downloads}, level control submits: {self.level_control_submits}, teacher lockout resets: {self.teacher_lockout_resets}, indy lockout resets: {self.indy_lockout_resets}, school student lockout resets: {self.school_student_lockout_resets}"
+        return f"Activity on {self.date}: CSV clicks: {self.csv_click_count}, login cards clicks: {self.login_cards_click_count}, primary pack downloads: {self.primary_coding_club_downloads}, python pack downloads: {self.python_coding_club_downloads}, level control submits: {self.level_control_submits}, teacher lockout resets: {self.teacher_lockout_resets}, indy lockout resets: {self.indy_lockout_resets}, school student lockout resets: {self.school_student_lockout_resets}, unverified teachers anonymised: {self.anonymised_unverified_teachers}, unverified independents anonymised: {self.anonymised_unverified_independents}"
 
 
 class TotalActivity(models.Model):
@@ -362,6 +364,8 @@ class TotalActivity(models.Model):
     teacher_registrations = models.PositiveIntegerField(default=0)
     student_registrations = models.PositiveIntegerField(default=0)
     independent_registrations = models.PositiveIntegerField(default=0)
+    anonymised_unverified_teachers = models.PositiveIntegerField(default=0)
+    anonymised_unverified_independents = models.PositiveIntegerField(default=0)
 
     class Meta:
         verbose_name_plural = "Total activity"

cfl_common/common/tests/test_models.py

@@ -105,5 +105,5 @@ def test_daily_activity_serializer(self):
 
         assert (
             str(daily_activity)
-            == f"Activity on {daily_activity.date}: CSV clicks: 0, login cards clicks: 0, primary pack downloads: 0, python pack downloads: 0, level control submits: 0, teacher lockout resets: 0, indy lockout resets: 0, school student lockout resets: 0"
+            == f"Activity on {daily_activity.date}: CSV clicks: 0, login cards clicks: 0, primary pack downloads: 0, python pack downloads: 0, level control submits: 0, teacher lockout resets: 0, indy lockout resets: 0, school student lockout resets: 0, unverified teachers anonymised: 0, unverified independents anonymised: 0"
         )

portal/tests/test_views.py

@@ -1,13 +1,23 @@
 import csv
 import io
 import json
-from datetime import timedelta, date
+from datetime import timedelta, date, datetime
 from unittest.mock import patch, Mock, ANY
 
 import PyPDF2
 import pytest
 from aimmo.models import Game
-from common.models import Teacher, UserSession, Student, Class, DailyActivity, School, UserProfile, TotalActivity
+from common.helpers.emails import NOTIFICATION_EMAIL
+from common.models import (
+    Teacher,
+    UserSession,
+    Student,
+    Class,
+    DailyActivity,
+    School,
+    UserProfile,
+    TotalActivity,
+)
 from common.tests.utils.classes import create_class_directly
 from common.tests.utils.organisation import create_organisation_directly, join_teacher_to_organisation
 from common.tests.utils.student import (
@@ -25,17 +35,16 @@
 from game.tests.utils.level import create_save_level
 from rest_framework.test import APIClient, APITestCase
 
-from cfl_common.common.helpers.emails import NOTIFICATION_EMAIL
 from deploy import captcha
 from portal.templatetags.app_tags import is_logged_in_as_admin_teacher
 from portal.views.api import anonymise
+from portal.views.cron.user import USER_DELETE_UNVERIFIED_ACCOUNT_DAYS
 from portal.views.teacher.teach import (
     REMINDER_CARDS_PDF_ROWS,
     REMINDER_CARDS_PDF_COLUMNS,
     REMINDER_CARDS_PDF_WARNING_TEXT,
     count_student_details_click,
 )
-from portal.views.cron.user import USER_DELETE_UNVERIFIED_ACCOUNT_DAYS
 
 
 class TestTeacherViews(TestCase):
@@ -925,7 +934,7 @@ def test_second_verify_email_reminder_view(self, send_email: Mock):
             assert_called=False,
         )
 
-    def test_delete_unverified_accounts_view(self):
+    def test_anonymise_unverified_accounts_view(self):
         now = timezone.now()
 
         for user in [self.teacher_user, self.indy_user, self.student_user]:
@@ -936,10 +945,10 @@ def test_delete_unverified_accounts_view(self):
             user_profile.is_verified = True
             user_profile.save()
 
-        def delete_unverified_users(
+        def anonymise_unverified_users(
             days: int,
             is_verified: bool,
-            assert_exists: bool,
+            assert_active: bool,
         ):
             date_joined = now - timedelta(days=days, hours=12)
 
@@ -994,45 +1003,61 @@ def delete_unverified_users(
                 new_user=indy_user,
             )
 
-            self.client.get(reverse("delete-unverified-accounts"))
+            activity_today = DailyActivity.objects.get_or_create(date=datetime.now().date())[0]
+            daily_teacher_count = activity_today.anonymised_unverified_teachers
+            daily_indy_count = activity_today.anonymised_unverified_independents
+
+            total_activity = TotalActivity.objects.get(id=1)
+            total_teacher_count = total_activity.anonymised_unverified_teachers
+            total_indy_count = total_activity.anonymised_unverified_independents
+
+            self.client.get(reverse("anonymise-unverified-accounts"))
+
+            # Assert the verified users exist
+            assert User.objects.get(id=self.teacher_user.id).is_active
+            assert User.objects.get(id=self.student_user.id).is_active
+            assert User.objects.get(id=self.indy_user.id).is_active
+
+            teacher_user_active = User.objects.get(id=teacher_user.id).is_active
+            indy_user_active = User.objects.get(id=indy_user.id).is_active
+            student_user_active = User.objects.get(id=student_user.id).is_active
+
+            assert teacher_user_active == assert_active
+            assert indy_user_active == assert_active
+            assert student_user_active
 
-            # Assert the verified users and teach
-            assert User.objects.filter(id=self.teacher_user.id).exists()
-            assert User.objects.filter(id=self.student_user.id).exists()
-            assert User.objects.filter(id=self.indy_user.id).exists()
+            activity_today = DailyActivity.objects.get_or_create(date=datetime.now().date())[0]
+            total_activity = TotalActivity.objects.get(id=1)
 
-            teacher_user_exists = User.objects.filter(id=teacher_user.id).exists()
-            indy_user_exists = User.objects.filter(id=indy_user.id).exists()
-            student_user_exists = User.objects.filter(id=student_user.id).exists()
+            if not teacher_user_active:
+                assert activity_today.anonymised_unverified_teachers == daily_teacher_count + 1
+                assert total_activity.anonymised_unverified_teachers == total_teacher_count + 1
 
-            assert teacher_user_exists == assert_exists
-            assert indy_user_exists == assert_exists
-            assert student_user_exists
+            if not indy_user_active:
+                assert activity_today.anonymised_unverified_independents == daily_indy_count + 1
+                assert total_activity.anonymised_unverified_independents == total_indy_count + 1
 
-            if teacher_user_exists:
-                teacher_user.delete()
-            if indy_user_exists:
-                indy_user.delete()
-            if student_user_exists:
-                student_user.delete()
+            teacher_user.delete()
+            indy_user.delete()
+            student_user.delete()
 
-        delete_unverified_users(
+        anonymise_unverified_users(
             days=USER_DELETE_UNVERIFIED_ACCOUNT_DAYS - 1,
             is_verified=False,
-            assert_exists=True,
+            assert_active=True,
         )
-        delete_unverified_users(
+        anonymise_unverified_users(
             days=USER_DELETE_UNVERIFIED_ACCOUNT_DAYS,
             is_verified=False,
-            assert_exists=False,
+            assert_active=False,
         )
-        delete_unverified_users(
+        anonymise_unverified_users(
             days=USER_DELETE_UNVERIFIED_ACCOUNT_DAYS,
             is_verified=True,
-            assert_exists=True,
+            assert_active=True,
         )
-        delete_unverified_users(
+        anonymise_unverified_users(
             days=USER_DELETE_UNVERIFIED_ACCOUNT_DAYS + 1,
             is_verified=False,
-            assert_exists=False,
+            assert_active=False,
         )

portal/urls.py

@@ -127,8 +127,8 @@
                             ),
                             path(
                                 "unverified/delete/",
-                                cron.user.DeleteUnverifiedAccounts.as_view(),
-                                name="delete-unverified-accounts",
+                                cron.user.AnonymiseUnverifiedAccounts.as_view(),
+                                name="anonymise-unverified-accounts",
                             ),
                         ]
                     ),