From de265e121ec3703075aefaa980f754bd00b52ae3 Mon Sep 17 00:00:00 2001 From: faucomte97 Date: Tue, 12 Sep 2023 13:26:14 +0100 Subject: [PATCH 1/5] feat: Keep track of deleted unverified users --- Pipfile.lock | 72 +++++++++---------- .../migrations/0044_update_activity_models.py | 33 +++++++++ cfl_common/common/models.py | 8 ++- portal/tests/test_views.py | 34 +++++++-- portal/views/cron/user.py | 39 ++++++---- 5 files changed, 132 insertions(+), 54 deletions(-) create mode 100644 cfl_common/common/migrations/0044_update_activity_models.py diff --git a/Pipfile.lock b/Pipfile.lock index b49f59295..25840e7d1 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -306,11 +306,11 @@ }, "google-auth": { "hashes": [ - "sha256:164cba9af4e6e4e40c3a4f90a1a6c12ee56f14c0b4868d1ca91b32826ab334ce", - "sha256:d61d1b40897407b574da67da1a833bdc10d5a11642566e506565d1b1a46ba873" + "sha256:2cec41407bd1e207f5b802638e32bb837df968bb5c05f413d0fa526fac4cf7a7", + "sha256:753a26312e6f1eaeec20bc6f2644a10926697da93446e1f8e24d6d32d45a922a" ], - "markers": "python_version >= '3.6'", - "version": "==2.22.0" + "markers": "python_version >= '3.7'", + "version": "==2.23.0" }, "greenlet": { "hashes": [ @@ -647,7 +647,7 @@ "sha256:0123cacc1627ae19ddf3c27a5de5bd67ee4586fbdd6440d9748f8abb483d3e86", "sha256:961d03dc3453ebbc59dbdea9e4e11c5651520a876d0f4db161e8674aae935da9" ], - "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2'", + "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'", "version": "==2.8.2" }, "pytz": { @@ -795,7 +795,7 @@ "sha256:1e61c37477a1626458e36f7b1d82aa5c9b094fa4802892072e49de9c60c4c926", "sha256:8abb2f1d86890a2dfb989f9a77cfcfd3e47c2a354b01111771326f8aa26e0254" ], - "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2'", + "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'", "version": "==1.16.0" }, "sortedcontainers": { @@ -854,11 +854,11 @@ }, "websocket-client": { "hashes": [ - "sha256:53e95c826bf800c4c465f50093a8c4ff091c7327023b10bfaff40cf1ef170eaa", - "sha256:ce54f419dfae71f4bdba69ebe65bf7f0a93fe71bc009ad3a010aacc3eebad537" + "sha256:3aad25d31284266bcfcfd1fd8a743f63282305a364b8d0948a43bd606acc652f", + "sha256:6cfc30d051ebabb73a5fa246efdcc14c8fbebbd0330f8984ac3bb6d9edd2ad03" ], "markers": "python_version >= '3.8'", - "version": "==1.6.2" + "version": "==1.6.3" }, "xlrd": { "hashes": [ @@ -894,31 +894,31 @@ }, "black": { "hashes": [ - "sha256:01ede61aac8c154b55f35301fac3e730baf0c9cf8120f65a9cd61a81cfb4a0c3", - "sha256:022a582720b0d9480ed82576c920a8c1dde97cc38ff11d8d8859b3bd6ca9eedb", - "sha256:25cc308838fe71f7065df53aedd20327969d05671bac95b38fdf37ebe70ac087", - "sha256:27eb7a0c71604d5de083757fbdb245b1a4fae60e9596514c6ec497eb63f95320", - "sha256:327a8c2550ddc573b51e2c352adb88143464bb9d92c10416feb86b0f5aee5ff6", - "sha256:47e56d83aad53ca140da0af87678fb38e44fd6bc0af71eebab2d1f59b1acf1d3", - "sha256:501387a9edcb75d7ae8a4412bb8749900386eaef258f1aefab18adddea1936bc", - "sha256:552513d5cd5694590d7ef6f46e1767a4df9af168d449ff767b13b084c020e63f", - "sha256:5c4bc552ab52f6c1c506ccae05681fab58c3f72d59ae6e6639e8885e94fe2587", - "sha256:642496b675095d423f9b8448243336f8ec71c9d4d57ec17bf795b67f08132a91", - "sha256:6d1c6022b86f83b632d06f2b02774134def5d4d4f1dac8bef16d90cda18ba28a", - "sha256:7f3bf2dec7d541b4619b8ce526bda74a6b0bffc480a163fed32eb8b3c9aed8ad", - "sha256:831d8f54c3a8c8cf55f64d0422ee875eecac26f5f649fb6c1df65316b67c8926", - "sha256:8417dbd2f57b5701492cd46edcecc4f9208dc75529bcf76c514864e48da867d9", - "sha256:86cee259349b4448adb4ef9b204bb4467aae74a386bce85d56ba4f5dc0da27be", - "sha256:893695a76b140881531062d48476ebe4a48f5d1e9388177e175d76234ca247cd", - "sha256:9fd59d418c60c0348505f2ddf9609c1e1de8e7493eab96198fc89d9f865e7a96", - "sha256:ad0014efc7acf0bd745792bd0d8857413652979200ab924fbf239062adc12491", - "sha256:b5b0ee6d96b345a8b420100b7d71ebfdd19fab5e8301aff48ec270042cd40ac2", - "sha256:c333286dc3ddca6fdff74670b911cccedacb4ef0a60b34e491b8a67c833b343a", - "sha256:f9062af71c59c004cd519e2fb8f5d25d39e46d3af011b41ab43b9c74e27e236f", - "sha256:fb074d8b213749fa1d077d630db0d5f8cc3b2ae63587ad4116e8a436e9bbe995" + "sha256:031e8c69f3d3b09e1aa471a926a1eeb0b9071f80b17689a655f7885ac9325a6f", + "sha256:13a2e4a93bb8ca74a749b6974925c27219bb3df4d42fc45e948a5d9feb5122b7", + "sha256:13ef033794029b85dfea8032c9d3b92b42b526f1ff4bf13b2182ce4e917f5100", + "sha256:14f04c990259576acd093871e7e9b14918eb28f1866f91968ff5524293f9c573", + "sha256:24b6b3ff5c6d9ea08a8888f6977eae858e1f340d7260cf56d70a49823236b62d", + "sha256:403397c033adbc45c2bd41747da1f7fc7eaa44efbee256b53842470d4ac5a70f", + "sha256:50254ebfa56aa46a9fdd5d651f9637485068a1adf42270148cd101cdf56e0ad9", + "sha256:538efb451cd50f43aba394e9ec7ad55a37598faae3348d723b59ea8e91616300", + "sha256:638619a559280de0c2aa4d76f504891c9860bb8fa214267358f0a20f27c12948", + "sha256:6a3b50e4b93f43b34a9d3ef00d9b6728b4a722c997c99ab09102fd5efdb88325", + "sha256:6ccd59584cc834b6d127628713e4b6b968e5f79572da66284532525a042549f9", + "sha256:75a2dc41b183d4872d3a500d2b9c9016e67ed95738a3624f4751a0cb4818fe71", + "sha256:7d30ec46de88091e4316b17ae58bbbfc12b2de05e069030f6b747dfc649ad186", + "sha256:8431445bf62d2a914b541da7ab3e2b4f3bc052d2ccbf157ebad18ea126efb91f", + "sha256:8fc1ddcf83f996247505db6b715294eba56ea9372e107fd54963c7553f2b6dfe", + "sha256:a732b82747235e0542c03bf352c126052c0fbc458d8a239a94701175b17d4855", + "sha256:adc3e4442eef57f99b5590b245a328aad19c99552e0bdc7f0b04db6656debd80", + "sha256:c46767e8df1b7beefb0899c4a95fb43058fa8500b6db144f4ff3ca38eb2f6393", + "sha256:c619f063c2d68f19b2d7270f4cf3192cb81c9ec5bc5ba02df91471d0b88c4c5c", + "sha256:cf3a4d00e4cdb6734b64bf23cd4341421e8953615cba6b3670453737a72ec204", + "sha256:cf99f3de8b3273a8317681d8194ea222f10e0133a24a7548c73ce44ea1679377", + "sha256:d6bc09188020c9ac2555a498949401ab35bb6bf76d4e0f8ee251694664df6301" ], "index": "pypi", - "version": "==23.7.0" + "version": "==23.9.1" }, "certifi": { "hashes": [ @@ -1243,11 +1243,11 @@ }, "pytest": { "hashes": [ - "sha256:2f2301e797521b23e4d2585a0a3d7b5e50fdddaaf7e7d6773ea26ddb17c213ab", - "sha256:460c9a59b14e27c602eb5ece2e47bec99dc5fc5f6513cf924a7d03a578991b1f" + "sha256:1d881c6124e08ff0a1bb75ba3ec0bfd8b5354a01c194ddd5a0a870a48d99b002", + "sha256:a766259cfab564a2ad52cb1aae1b881a75c3eb7e34ca3779697c23ed47c47069" ], "index": "pypi", - "version": "==7.4.1" + "version": "==7.4.2" }, "pytest-cov": { "hashes": [ @@ -1368,7 +1368,7 @@ "sha256:1e61c37477a1626458e36f7b1d82aa5c9b094fa4802892072e49de9c60c4c926", "sha256:8abb2f1d86890a2dfb989f9a77cfcfd3e47c2a354b01111771326f8aa26e0254" ], - "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2'", + "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'", "version": "==1.16.0" }, "snapshottest": { diff --git a/cfl_common/common/migrations/0044_update_activity_models.py b/cfl_common/common/migrations/0044_update_activity_models.py new file mode 100644 index 000000000..37962a203 --- /dev/null +++ b/cfl_common/common/migrations/0044_update_activity_models.py @@ -0,0 +1,33 @@ +# Generated by Django 3.2.20 on 2023-09-12 11:58 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('common', '0043_add_total_activity'), + ] + + operations = [ + migrations.AddField( + model_name='dailyactivity', + name='deleted_unverified_independents', + field=models.PositiveIntegerField(default=0), + ), + migrations.AddField( + model_name='dailyactivity', + name='deleted_unverified_teachers', + field=models.PositiveIntegerField(default=0), + ), + migrations.AddField( + model_name='totalactivity', + name='deleted_unverified_independents', + field=models.PositiveIntegerField(default=0), + ), + migrations.AddField( + model_name='totalactivity', + name='deleted_unverified_teachers', + field=models.PositiveIntegerField(default=0), + ), + ] diff --git a/cfl_common/common/models.py b/cfl_common/common/models.py index cacff07af..f0af97ca1 100644 --- a/cfl_common/common/models.py +++ b/cfl_common/common/models.py @@ -1,8 +1,8 @@ -import pgeocode import re from datetime import timedelta from uuid import uuid4 +import pgeocode from django.contrib.auth.models import User from django.db import models from django.utils import timezone @@ -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) + deleted_unverified_teachers = models.PositiveIntegerField(default=0) + deleted_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 deleted: {self.deleted_unverified_teachers}, unverified independents deleted: {self.deleted_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) + deleted_unverified_teachers = models.PositiveIntegerField(default=0) + deleted_unverified_independents = models.PositiveIntegerField(default=0) class Meta: verbose_name_plural = "Total activity" diff --git a/portal/tests/test_views.py b/portal/tests/test_views.py index 73898ab9b..86731579a 100644 --- a/portal/tests/test_views.py +++ b/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): @@ -994,6 +1003,14 @@ def delete_unverified_users( new_user=indy_user, ) + activity_today = DailyActivity.objects.get_or_create(date=datetime.now().date())[0] + daily_teacher_count = activity_today.deleted_unverified_teachers + daily_indy_count = activity_today.deleted_unverified_independents + + total_activity = TotalActivity.objects.get(id=1) + total_teacher_count = total_activity.deleted_unverified_teachers + total_indy_count = total_activity.deleted_unverified_independents + self.client.get(reverse("delete-unverified-accounts")) # Assert the verified users and teach @@ -1009,10 +1026,19 @@ def delete_unverified_users( assert indy_user_exists == assert_exists assert student_user_exists + activity_today = DailyActivity.objects.get_or_create(date=datetime.now().date())[0] + total_activity = TotalActivity.objects.get(id=1) + if teacher_user_exists: teacher_user.delete() + else: + assert activity_today.deleted_unverified_teachers == daily_teacher_count + 1 + assert total_activity.deleted_unverified_teachers == total_teacher_count + 1 if indy_user_exists: indy_user.delete() + else: + assert activity_today.deleted_unverified_independents == daily_teacher_count + 1 + assert total_activity.deleted_unverified_independents == total_indy_count + 1 if student_user_exists: student_user.delete() diff --git a/portal/views/cron/user.py b/portal/views/cron/user.py index e77d50895..0d1384965 100644 --- a/portal/views/cron/user.py +++ b/portal/views/cron/user.py @@ -1,19 +1,20 @@ import logging -from datetime import timedelta +from datetime import timedelta, datetime +from common.helpers.emails import ( + NOTIFICATION_EMAIL, + generate_token_for_email, + send_email, +) +from common.models import DailyActivity, TotalActivity from django.contrib.auth.models import User +from django.db.models import F from django.db.models.query import QuerySet from django.urls import reverse from django.utils import timezone from rest_framework.response import Response from rest_framework.views import APIView -from cfl_common.common.helpers.emails import ( - NOTIFICATION_EMAIL, - generate_token_for_email, - send_email, -) - from ...mixins import CronMixin # TODO: move email templates to DotDigital. @@ -38,7 +39,7 @@ USER_DELETE_UNVERIFIED_ACCOUNT_DAYS = 19 -def get_unverified_users(days: int, same_day: bool) -> QuerySet[User]: +def get_unverified_users(days: int, same_day: bool) -> (int, int, QuerySet[User]): now = timezone.now() # All expired unverified users. @@ -58,7 +59,11 @@ def get_unverified_users(days: int, same_day: bool) -> QuerySet[User]: new_student__class_field__isnull=True, ) - return teacher_queryset.union(independent_student_queryset) + return ( + teacher_queryset.count(), + independent_student_queryset.count(), + teacher_queryset.union(independent_student_queryset), + ) def build_absolute_google_uri(request, location: str) -> str: @@ -75,7 +80,7 @@ def build_absolute_google_uri(request, location: str) -> str: class FirstVerifyEmailReminderView(CronMixin, APIView): def get(self, request): - user_queryset = get_unverified_users( + _, _, user_queryset = get_unverified_users( USER_1ST_VERIFY_EMAIL_REMINDER_DAYS, same_day=True, ) @@ -122,7 +127,7 @@ def get(self, request): class SecondVerifyEmailReminderView(CronMixin, APIView): def get(self, request): - user_queryset = get_unverified_users( + _, _, user_queryset = get_unverified_users( USER_2ND_VERIFY_EMAIL_REMINDER_DAYS, same_day=True, ) @@ -171,7 +176,7 @@ class DeleteUnverifiedAccounts(CronMixin, APIView): def get(self, request): user_count = User.objects.count() - user_queryset = get_unverified_users( + teacher_count, indy_count, user_queryset = get_unverified_users( USER_DELETE_UNVERIFIED_ACCOUNT_DAYS, same_day=False, ) @@ -186,4 +191,14 @@ def get(self, request): user_count -= User.objects.count() logging.info(f"{user_count} unverified users deleted.") + activity_today = DailyActivity.objects.get_or_create(date=datetime.now().date())[0] + activity_today.deleted_unverified_teachers = teacher_count + activity_today.deleted_unverified_independents = indy_count + activity_today.save() + + TotalActivity.objects.update( + deleted_unverified_teachers=F("deleted_unverified_teachers") + teacher_count, + deleted_unverified_independents=F("deleted_unverified_independents") + indy_count, + ) + return Response() From acee2714d5cd022281d2e84a7ca4bd5740ae53ac Mon Sep 17 00:00:00 2001 From: faucomte97 Date: Tue, 12 Sep 2023 13:35:38 +0100 Subject: [PATCH 2/5] Fix test --- cfl_common/common/tests/test_models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfl_common/common/tests/test_models.py b/cfl_common/common/tests/test_models.py index f5168d5b4..bc41a8fc0 100644 --- a/cfl_common/common/tests/test_models.py +++ b/cfl_common/common/tests/test_models.py @@ -95,5 +95,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 deleted: 0, unverified independents deleted: 0" ) From 222726fb38d52f23037a1e7d93f165af72019cbe Mon Sep 17 00:00:00 2001 From: faucomte97 Date: Tue, 12 Sep 2023 15:51:19 +0100 Subject: [PATCH 3/5] Feedback --- portal/views/cron/user.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/portal/views/cron/user.py b/portal/views/cron/user.py index 0d1384965..dee538dcd 100644 --- a/portal/views/cron/user.py +++ b/portal/views/cron/user.py @@ -59,11 +59,7 @@ def get_unverified_users(days: int, same_day: bool) -> (int, int, QuerySet[User] new_student__class_field__isnull=True, ) - return ( - teacher_queryset.count(), - independent_student_queryset.count(), - teacher_queryset.union(independent_student_queryset), - ) + return teacher_queryset, independent_student_queryset def build_absolute_google_uri(request, location: str) -> str: @@ -80,10 +76,11 @@ def build_absolute_google_uri(request, location: str) -> str: class FirstVerifyEmailReminderView(CronMixin, APIView): def get(self, request): - _, _, user_queryset = get_unverified_users( + teacher_queryset, independent_student_queryset = get_unverified_users( USER_1ST_VERIFY_EMAIL_REMINDER_DAYS, same_day=True, ) + user_queryset = teacher_queryset.union(independent_student_queryset) user_count = user_queryset.count() logging.info(f"{user_count} emails unverified.") @@ -127,10 +124,11 @@ def get(self, request): class SecondVerifyEmailReminderView(CronMixin, APIView): def get(self, request): - _, _, user_queryset = get_unverified_users( + teacher_queryset, independent_student_queryset = get_unverified_users( USER_2ND_VERIFY_EMAIL_REMINDER_DAYS, same_day=True, ) + user_queryset = teacher_queryset.union(independent_student_queryset) user_count = user_queryset.count() logging.info(f"{user_count} emails unverified.") @@ -176,10 +174,14 @@ class DeleteUnverifiedAccounts(CronMixin, APIView): def get(self, request): user_count = User.objects.count() - teacher_count, indy_count, user_queryset = get_unverified_users( + teacher_queryset, independent_student_queryset = get_unverified_users( USER_DELETE_UNVERIFIED_ACCOUNT_DAYS, same_day=False, ) + teacher_count = teacher_queryset.count() + indy_count = independent_student_queryset.count() + + user_queryset = teacher_queryset.union(independent_student_queryset) for user in user_queryset.iterator(chunk_size=100): try: From 2cfde5e947cb7d63fb5fbbc0f980eadabfc4384c Mon Sep 17 00:00:00 2001 From: faucomte97 Date: Thu, 14 Sep 2023 17:50:09 +0100 Subject: [PATCH 4/5] Update cron job to anonymise users instead --- .../migrations/0044_update_activity_models.py | 10 +-- cfl_common/common/models.py | 10 +-- cfl_common/common/tests/test_models.py | 2 +- portal/tests/test_views.py | 77 +++++++++---------- portal/urls.py | 4 +- portal/views/cron/user.py | 21 ++--- 6 files changed, 62 insertions(+), 62 deletions(-) diff --git a/cfl_common/common/migrations/0044_update_activity_models.py b/cfl_common/common/migrations/0044_update_activity_models.py index 37962a203..ffdf9593f 100644 --- a/cfl_common/common/migrations/0044_update_activity_models.py +++ b/cfl_common/common/migrations/0044_update_activity_models.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.20 on 2023-09-12 11:58 +# Generated by Django 3.2.20 on 2023-09-14 16:26 from django.db import migrations, models @@ -12,22 +12,22 @@ class Migration(migrations.Migration): operations = [ migrations.AddField( model_name='dailyactivity', - name='deleted_unverified_independents', + name='anonymised_unverified_independents', field=models.PositiveIntegerField(default=0), ), migrations.AddField( model_name='dailyactivity', - name='deleted_unverified_teachers', + name='anonymised_unverified_teachers', field=models.PositiveIntegerField(default=0), ), migrations.AddField( model_name='totalactivity', - name='deleted_unverified_independents', + name='anonymised_unverified_independents', field=models.PositiveIntegerField(default=0), ), migrations.AddField( model_name='totalactivity', - name='deleted_unverified_teachers', + name='anonymised_unverified_teachers', field=models.PositiveIntegerField(default=0), ), ] diff --git a/cfl_common/common/models.py b/cfl_common/common/models.py index f0af97ca1..704a741ff 100644 --- a/cfl_common/common/models.py +++ b/cfl_common/common/models.py @@ -345,14 +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) - deleted_unverified_teachers = models.PositiveIntegerField(default=0) - deleted_unverified_independents = 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}, unverified teachers deleted: {self.deleted_unverified_teachers}, unverified independents deleted: {self.deleted_unverified_independents}" + 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): @@ -364,8 +364,8 @@ class TotalActivity(models.Model): teacher_registrations = models.PositiveIntegerField(default=0) student_registrations = models.PositiveIntegerField(default=0) independent_registrations = models.PositiveIntegerField(default=0) - deleted_unverified_teachers = models.PositiveIntegerField(default=0) - deleted_unverified_independents = 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" diff --git a/cfl_common/common/tests/test_models.py b/cfl_common/common/tests/test_models.py index bc41a8fc0..949350d2d 100644 --- a/cfl_common/common/tests/test_models.py +++ b/cfl_common/common/tests/test_models.py @@ -95,5 +95,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, unverified teachers deleted: 0, unverified independents deleted: 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" ) diff --git a/portal/tests/test_views.py b/portal/tests/test_views.py index 86731579a..c7428a7d1 100644 --- a/portal/tests/test_views.py +++ b/portal/tests/test_views.py @@ -934,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]: @@ -945,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) @@ -1004,61 +1004,60 @@ def delete_unverified_users( ) activity_today = DailyActivity.objects.get_or_create(date=datetime.now().date())[0] - daily_teacher_count = activity_today.deleted_unverified_teachers - daily_indy_count = activity_today.deleted_unverified_independents + 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.deleted_unverified_teachers - total_indy_count = total_activity.deleted_unverified_independents + total_teacher_count = total_activity.anonymised_unverified_teachers + total_indy_count = total_activity.anonymised_unverified_independents - self.client.get(reverse("delete-unverified-accounts")) + self.client.get(reverse("anonymise-unverified-accounts")) - # 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() + # 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_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() + 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_exists == assert_exists - assert indy_user_exists == assert_exists - assert student_user_exists + assert teacher_user_active == assert_active + assert indy_user_active == assert_active + assert student_user_active activity_today = DailyActivity.objects.get_or_create(date=datetime.now().date())[0] total_activity = TotalActivity.objects.get(id=1) - if teacher_user_exists: - teacher_user.delete() - else: - assert activity_today.deleted_unverified_teachers == daily_teacher_count + 1 - assert total_activity.deleted_unverified_teachers == total_teacher_count + 1 - if indy_user_exists: - indy_user.delete() - else: - assert activity_today.deleted_unverified_independents == daily_teacher_count + 1 - assert total_activity.deleted_unverified_independents == total_indy_count + 1 - if student_user_exists: - student_user.delete() - - delete_unverified_users( + 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 + + 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 + + teacher_user.delete() + indy_user.delete() + student_user.delete() + + 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, ) diff --git a/portal/urls.py b/portal/urls.py index 4d39eae01..17692cde3 100644 --- a/portal/urls.py +++ b/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", ), ] ), diff --git a/portal/views/cron/user.py b/portal/views/cron/user.py index dee538dcd..a47373901 100644 --- a/portal/views/cron/user.py +++ b/portal/views/cron/user.py @@ -14,6 +14,7 @@ from django.utils import timezone from rest_framework.response import Response from rest_framework.views import APIView +from portal.views.api import anonymise from ...mixins import CronMixin @@ -170,9 +171,9 @@ def get(self, request): return Response() -class DeleteUnverifiedAccounts(CronMixin, APIView): +class AnonymiseUnverifiedAccounts(CronMixin, APIView): def get(self, request): - user_count = User.objects.count() + user_count = User.objects.filter(is_active=True).count() teacher_queryset, independent_student_queryset = get_unverified_users( USER_DELETE_UNVERIFIED_ACCOUNT_DAYS, @@ -185,22 +186,22 @@ def get(self, request): for user in user_queryset.iterator(chunk_size=100): try: - user.delete() + anonymise(user) except Exception as ex: - logging.error(f"Failed to delete user with id: {user.id}") + logging.error(f"Failed to anonymise user with id: {user.id}") logging.exception(ex) - user_count -= User.objects.count() - logging.info(f"{user_count} unverified users deleted.") + user_count -= User.objects.filter(is_active=True).count() + logging.info(f"{user_count} unverified users anonymised.") activity_today = DailyActivity.objects.get_or_create(date=datetime.now().date())[0] - activity_today.deleted_unverified_teachers = teacher_count - activity_today.deleted_unverified_independents = indy_count + activity_today.anonymised_unverified_teachers = teacher_count + activity_today.anonymised_unverified_independents = indy_count activity_today.save() TotalActivity.objects.update( - deleted_unverified_teachers=F("deleted_unverified_teachers") + teacher_count, - deleted_unverified_independents=F("deleted_unverified_independents") + indy_count, + anonymised_unverified_teachers=F("anonymised_unverified_teachers") + teacher_count, + anonymised_unverified_independents=F("anonymised_unverified_independents") + indy_count, ) return Response() From e3905e2a4e7b4d6ef4a6e66a74897998994a0071 Mon Sep 17 00:00:00 2001 From: faucomte97 Date: Tue, 19 Sep 2023 20:56:38 +0100 Subject: [PATCH 5/5] Update return type --- portal/views/cron/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/portal/views/cron/user.py b/portal/views/cron/user.py index a47373901..00bbb654d 100644 --- a/portal/views/cron/user.py +++ b/portal/views/cron/user.py @@ -40,7 +40,7 @@ USER_DELETE_UNVERIFIED_ACCOUNT_DAYS = 19 -def get_unverified_users(days: int, same_day: bool) -> (int, int, QuerySet[User]): +def get_unverified_users(days: int, same_day: bool) -> (QuerySet[User], QuerySet[User]): now = timezone.now() # All expired unverified users.