Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Keep track of deleted unverified users #2174

Merged
merged 6 commits into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 36 additions & 36 deletions Pipfile.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 33 additions & 0 deletions cfl_common/common/migrations/0044_update_activity_models.py
Original file line number Diff line number Diff line change
@@ -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),
),
]
6 changes: 5 additions & 1 deletion cfl_common/common/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion cfl_common/common/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
89 changes: 57 additions & 32 deletions portal/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -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 (
Expand All @@ -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):
Expand Down Expand Up @@ -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]:
Expand All @@ -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)

Expand Down Expand Up @@ -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,
)
4 changes: 2 additions & 2 deletions portal/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
),
]
),
Expand Down
Loading