Skip to content

Commit

Permalink
User cleanup (TampereHacklab#459)
Browse files Browse the repository at this point in the history
* add mark_for_deletion action and filter to django admin for users

* add setting for how long to wait untill deleting, process the signal and send email, add management command

* bit of testing

* black
  • Loading branch information
tswfi committed Dec 22, 2023
1 parent 408ad0e commit 97f3869
Show file tree
Hide file tree
Showing 7 changed files with 263 additions and 4 deletions.
5 changes: 5 additions & 0 deletions drfx/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@
"*",
]


# After user has been marked for deletion, how many days to wait until
# really deleting the user and their associated data
USER_DELETION_DAYS = 90

# https://docs.djangoproject.com/en/4.1/ref/settings/
SECURE_PROXY_SSL_HEADER = ("HTTP_X_FORWARDED_PROTO", "https")

Expand Down
23 changes: 19 additions & 4 deletions users/admin.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
from django.contrib import admin
from django.contrib.auth.admin import UserAdmin
from django.utils import timezone

from rangefilter.filters import DateRangeFilter

from .filters import PredefAgeListFilter
from .filters import PredefAgeListFilter, MarkedForDeletionFilter
from .forms import CustomUserChangeForm, CustomUserCreationForm
from .models import (
BankTransaction,
Expand All @@ -28,6 +29,7 @@ class CustomUserAdmin(UserAdmin):
add_form = CustomUserCreationForm
form = CustomUserChangeForm
model = CustomUser

ordering = (
"first_name",
"last_name",
Expand All @@ -38,9 +40,7 @@ class CustomUserAdmin(UserAdmin):
"last_name",
"nick",
"mxid",
"language",
"municipality",
"age_years",
"marked_for_deletion_on",
"is_active",
"is_staff",
"is_superuser",
Expand All @@ -49,6 +49,7 @@ class CustomUserAdmin(UserAdmin):
list_filter = (
"is_active",
"is_staff",
MarkedForDeletionFilter,
"language",
"municipality",
PredefAgeListFilter,
Expand Down Expand Up @@ -88,6 +89,20 @@ class CustomUserAdmin(UserAdmin):
)
inlines = [ServiceSubscriptionInline]

actions = ["mark_for_deletion_on", "mark_for_deletion_off"]

def mark_for_deletion_on(self, request, queryset):
queryset.update(marked_for_deletion_on=timezone.now())

mark_for_deletion_on.short_description = "Mark selected users for deletion"

def mark_for_deletion_off(self, request, queryset):
queryset.update(marked_for_deletion_on=None)

mark_for_deletion_off.short_description = (
"Remove mark for deletion from selected users"
)


class NFCCardAdmin(admin.ModelAdmin):
list_display = [
Expand Down
23 changes: 23 additions & 0 deletions users/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,29 @@ def _add_years(self, dt, years):
return dt


class MarkedForDeletionFilter(admin.SimpleListFilter):
title = _("Marked for deletion")
parameter_name = "marked_for_deletion_on__isnull"

def lookups(self, request, model_admin):
"""
Few predefined filters
"""
return (
("false", _("Marked for deletion")),
("true", _("NOT Marked for deletion")),
)

def queryset(self, request, queryset):
value = self.value()
if value == "true":
return queryset.filter(marked_for_deletion_on__isnull=True)
elif value == "false":
return queryset.filter(marked_for_deletion_on__isnull=False)

return queryset


class UserFilter(filters.FilterSet):
class Meta:
model = models.CustomUser
Expand Down
31 changes: 31 additions & 0 deletions users/management/commands/delete_marked_users.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import logging
from django.core.management.base import BaseCommand
from django.utils import timezone

from users.models import CustomUser

from drfx import config

logger = logging.getLogger(__name__)


class Command(BaseCommand):
help = "Delete all users that have been marked for deletion for more than the cutoff setting"

def handle(self, *args, **options):
# some safety margin
dt = timezone.now() - timezone.timedelta(days=config.USER_DELETION_DAYS)

logger.info(
f" Search for users that have been marked for deletion for over {config.USER_DELETION_DAYS} days"
)

users = CustomUser.objects.filter(
marked_for_deletion_on__isnull=False, marked_for_deletion_on__lt=dt
)

for user in users:
logger.info(
f" Deleting User {user} as it has been marked for deletion over {config.USER_DELETION_DAYS} days"
)
user.delete()
45 changes: 45 additions & 0 deletions users/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,51 @@ def send_user_activated(sender, instance: models.CustomUser, raw, **kwargs):
logger.info("User deactivation done {}".format(instance))


@receiver(pre_save, sender=models.CustomUser)
def handle_marked_for_deletion(sender, instance: models.CustomUser, raw, **kwargs):
"""
Send email to user that their account has now been marked for deletion and
deactivated and will be completely removed after a while
"""
# do nothing for raw or created signals
if raw:
return

# if our value didn't change then nothign to be done
try:
previous = models.CustomUser.objects.get(id=instance.id)
except models.CustomUser.DoesNotExist:
# new user, it wont be activated or deactived yet
return

# already set, no need to do again
if previous.marked_for_deletion_on:
return

if instance.marked_for_deletion_on:
# so the field was changed, mark the user is_active=False and send and email too
logger.info(
"User marked for deletion, also changing active to false and informing the user {}".format(
instance
)
)
instance.is_active = False

# and send the email
context = {"user": instance, "config": config}

translation.activate(instance.language)
# TODO: maybe move this subject to settings?
subject = _("Your account has been deactivated and marked for deletion")
from_email = config.NOREPLY_FROM_ADDRESS
to = [instance.email, config.MEMBERSHIP_APPLICATION_NOTIFY_ADDRESS]
plaintext_content = render_to_string(
"mail/account_deactivated_and_marked_for_deletion.txt", context
)

send_mail(subject, plaintext_content, from_email, to)


#
# Signal door access denied
#
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{% extends 'mail/email_base.txt' %}
{% load i18n %}
{% block content %}

{% blocktrans %}Your account has been deactivated and marked for deletion{% endblocktrans %}

{{user.first_name}} {{user.last_name}}

{# TODO: tell what this means and how to recover if you want to #}
{% endblock %}
130 changes: 130 additions & 0 deletions users/tests/test_user_deletion.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
from django.contrib.auth import get_user_model
from django.core import mail
from django.test import TestCase
from django.utils import timezone
from drfx import config

from .. import models


class TestUserDeletion(TestCase):
def setUp(self):
self.user = get_user_model().objects.create_customuser(
first_name="FirstName",
last_name="LastName",
email="[email protected]",
birthday=timezone.now(),
municipality="City",
nick="user1",
phone="+358123123",
)

self.memberservice = models.MemberService.objects.create(
name="TestService", cost=10, days_per_payment=30, days_before_warning=2
)

mail.outbox = []

def test_mark_user_for_deletion(self):
"""
Test marking user for deletion
"""
self.assertEqual(self.user.is_active, True)
self.assertIsNone(self.user.marked_for_deletion_on)

self.user.marked_for_deletion_on = timezone.now()
self.user.save()

# user was marked is inactive
self.assertEqual(self.user.is_active, False)

# email was sent to user and admins
self.assertEqual(len(mail.outbox), 1)
self.assertIn(self.user.email, mail.outbox[0].to)
self.assertIn(config.MEMBERSHIP_APPLICATION_NOTIFY_ADDRESS, mail.outbox[0].to)
self.assertIn("Your account", mail.outbox[0].subject)
self.assertIn("deletion", mail.outbox[0].body)
self.assertIn(self.user.first_name, mail.outbox[0].body)

# clear outbox
mail.outbox = []

# mark again
self.user.marked_for_deletion_on = timezone.now()
self.user.save()

# no emails sent
self.assertEqual(len(mail.outbox), 0)

# clear again
mail.outbox = []

# remove mark
self.user.marked_for_deletion_on = None
self.user.save()

# user still marked as inactive
self.assertEqual(self.user.is_active, False)

# no email notifications
self.assertEqual(len(mail.outbox), 0)

def test_user_deletion(self):
"""
Check that all necessary (and just the necessary) objects are cleared when user is deleted
"""
# add few objects pointing to this user, log entries, payment transactions, service subcsriptions
self.subscription = models.ServiceSubscription.objects.create(
user=self.user,
service=self.memberservice,
state=models.ServiceSubscription.ACTIVE,
paid_until=timezone.now().date(),
)

# log entry for the user
models.UsersLog.objects.create(user=self.user, message="test")

# nfc card
models.NFCCard.objects.create(user=self.user, cardid="123")

# custom invoice
models.CustomInvoice.objects.create(
user=self.user,
subscription=self.subscription,
days=10,
amount=10,
)

# bank transaction that points to the user
self.transaction = models.BankTransaction.objects.create(
user=self.user, date=timezone.now().date(), amount=10
)

# delete the user
self.user.delete()

# user was deleted
self.assertEqual(models.CustomUser.objects.count(), 0)

# the subscription was deleted
self.assertEqual(models.ServiceSubscription.objects.count(), 0)

# message log was deleted
self.assertEqual(models.UsersLog.objects.count(), 0)

# nfc card is removed
self.assertEqual(models.NFCCard.objects.count(), 0)

# custom invoice is removed
self.assertEqual(models.CustomInvoice.objects.count(), 0)

# bank transaction was not removed
self.assertEqual(models.BankTransaction.objects.count(), 1)
# but the user assignment is cleared
self.transaction.refresh_from_db()
self.assertEqual(self.transaction.user, None)

def tearDown(self):
mail.outbox = []
get_user_model().objects.all().delete()
models.MemberService.objects.all().delete()

0 comments on commit 97f3869

Please sign in to comment.