Skip to content
Draft
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
63 changes: 63 additions & 0 deletions rodatraden/admin.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
from django.contrib import admin
from django.contrib.auth.models import User
from django.contrib.auth.admin import UserAdmin
from django.utils.translation import gettext_lazy as _, ngettext
from django.utils import timezone
from django.contrib import messages
from django.conf import settings
from django.db import models
from datetime import timedelta

from .models import *

Expand All @@ -20,3 +28,58 @@

# Register all the models in myModels to the admin site
admin.site.register(myModels)


def delete_inactive_users(modeladmin, request, queryset):
"""Admin action to delete inactive users based on settings."""
years = getattr(settings, 'INACTIVE_USER_AUTODELETE_YEARS', 5)
if years <= 0:
modeladmin.message_user(
request,
_("Inactive user auto-deletion is disabled (INACTIVE_USER_AUTODELETE_YEARS is 0 or not set)."),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix inaccurate disabled-state message

The text says “0 or not set”, but getattr(settings, 'INACTIVE_USER_AUTODELETE_YEARS', 5) treats “not set” as enabled (5). Update the message to match the real condition (<= 0).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rodatraden/admin.py` at line 39, The message string currently claims "0 or
not set" which is incorrect because the code uses getattr(settings,
'INACTIVE_USER_AUTODELETE_YEARS', 5) and thus treats "not set" as enabled;
update the message text where it's defined (the tuple/list entry containing
_("Inactive user auto-deletion is disabled (INACTIVE_USER_AUTODELETE_YEARS is 0
or not set).")) to accurately reflect the real disabled condition (<= 0) — e.g.,
change it to _("Inactive user auto-deletion is disabled
(INACTIVE_USER_AUTODELETE_YEARS is <= 0).") so the message matches the behavior
driven by settings.INACTIVE_USER_AUTODELETE_YEARS.

messages.WARNING
)
return

threshold_date = timezone.now() - timedelta(days=years * 365)

# Find inactive users: exclude staff and superusers
inactive_users = queryset.filter(
is_staff=False,
is_superuser=False
).filter(
models.Q(last_login__lt=threshold_date) |
models.Q(last_login__isnull=True, date_joined__lt=threshold_date)
)
Comment thread
it-amanuens marked this conversation as resolved.

count = inactive_users.count()
if count == 0:
modeladmin.message_user(
request,
_("No inactive users found to delete."),
messages.INFO
)
return

inactive_users.delete()
modeladmin.message_user(
request,
ngettext(
"%d inactive user was successfully deleted.",
"%d inactive users were successfully deleted.",
count,
) % count,
messages.SUCCESS,
)


delete_inactive_users.short_description = _("Delete inactive users")


class CustomUserAdmin(UserAdmin):
actions = [delete_inactive_users] + list(UserAdmin.actions)
Comment on lines +76 to +80

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

cat -n rodatraden/admin.py | head -100

Repository: it-amanuens/rodatraden

Length of output: 3019


🏁 Script executed:

rg "allowed_permissions" --type py -i

Repository: it-amanuens/rodatraden

Length of output: 48


🏁 Script executed:

fd -e txt -e txt -e cfg -e ini | grep -i req

Repository: it-amanuens/rodatraden

Length of output: 83


🏁 Script executed:

cat requirements.txt

Repository: it-amanuens/rodatraden

Length of output: 817


🌐 Web query:

Django admin action allowed_permissions attribute Django 6.0

💡 Result:

In Django 6.0, the admin action function attribute you should use is action_func.allowed_permissions (not ModelAdmin.allowed_permissions). You set it via the @admin.action decorator (preferred) or by assigning allowed_permissions on the action function directly. Example (Django 6.0 docs): @admin.action(permissions=["publish"], description="Mark selected stories as published") def make_published(self, request, queryset): queryset.update(status="p") The docs also state this is equivalent to: def make_published(self, request, queryset): queryset.update(status="p") make_published.allowed_permissions = ["publish"] make_published.short_description = "Mark selected stories as published" [1]. How permissions are checked: - Django documents that passing permissions to the action decorator limits the action’s availability based on permission checks on the ModelAdmin, and that the make_published action is only shown to users who pass the relevant ModelAdmin.has_*permission check [1]. - The available permissions values map to these methods: 'add' -> has_add_permission, 'change' -> has_change_permission, 'delete' -> has_delete_permission, 'view' -> has_view_permission [1]. - The docs also note you can use other values if you implement a corresponding has(self, request) method on ModelAdmin [1]. Note: There is no separate/alternate “Django admin action allowed_permissions attribute” for Django 6.0 beyond the allowed_permissions attribute on the action function described above; it is part of the action decorator/action-function API documented for 6.0 [1].

Citations:


Require delete permission for this destructive action

The delete_inactive_users action performs hard deletes (line 64) but does not declare required permissions. Without allowed_permissions, any user with change access to the User model can execute this destructive action. Add explicit permission requirements to restrict access.

Suggested fix
delete_inactive_users.short_description = _("Delete inactive users")
+delete_inactive_users.allowed_permissions = ("delete",)

class CustomUserAdmin(UserAdmin):
    actions = [delete_inactive_users] + list(UserAdmin.actions)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
delete_inactive_users.short_description = _("Delete inactive users")
class CustomUserAdmin(UserAdmin):
actions = [delete_inactive_users] + list(UserAdmin.actions)
delete_inactive_users.short_description = _("Delete inactive users")
delete_inactive_users.allowed_permissions = ("delete",)
class CustomUserAdmin(UserAdmin):
actions = [delete_inactive_users] + list(UserAdmin.actions)
🧰 Tools
🪛 Ruff (0.15.12)

[warning] 80-80: Consider [delete_inactive_users, *list(UserAdmin.actions)] instead of concatenation

Replace with [delete_inactive_users, *list(UserAdmin.actions)]

(RUF005)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rodatraden/admin.py` around lines 76 - 80, The admin action
delete_inactive_users currently performs hard deletes but lacks permission
restrictions; add an explicit allowed_permissions attribute to the action (e.g.,
set delete_inactive_users.allowed_permissions = ('delete',)) so only users with
the User model delete permission can execute it, and keep the
CustomUserAdmin.actions = [delete_inactive_users] + list(UserAdmin.actions)
registration intact.



# Unregister the default User admin and register our custom one
admin.site.unregister(User)
admin.site.register(User, CustomUserAdmin)
96 changes: 94 additions & 2 deletions rodatraden/tests.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,95 @@
from django.test import TestCase
from django.test import TestCase, RequestFactory

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

git ls-files rodatraden/tests.py

Repository: it-amanuens/rodatraden

Length of output: 86


🏁 Script executed:

fd tests.py

Repository: it-amanuens/rodatraden

Length of output: 86


🏁 Script executed:

cat -n rodatraden/tests.py | head -100

Repository: it-amanuens/rodatraden

Length of output: 4355


Use @override_settings decorator to avoid test isolation issues

Tests directly mutate settings.INACTIVE_USER_AUTODELETE_YEARS and restore it at method end (lines 71-72, 94-95). If an assertion fails before restoration, the setting leaks into subsequent tests. Use Django's @override_settings decorator instead for guaranteed cleanup.

Import override_settings from django.test and apply it to both test methods:

Suggested fix
-from django.test import TestCase, RequestFactory
+from django.test import TestCase, RequestFactory, override_settings
...
-    def test_delete_inactive_users(self):
-        # Set the threshold to 5 years
-        original_setting = getattr(settings, 'INACTIVE_USER_AUTODELETE_YEARS', 5)
-        settings.INACTIVE_USER_AUTODELETE_YEARS = 5
+    `@override_settings`(INACTIVE_USER_AUTODELETE_YEARS=5)
+    def test_delete_inactive_users(self):
...
-        # Restore setting
-        settings.INACTIVE_USER_AUTODELETE_YEARS = original_setting

-    def test_delete_inactive_users_disabled(self):
-        # Set to disabled
-        original_setting = getattr(settings, 'INACTIVE_USER_AUTODELETE_YEARS', 5)
-        settings.INACTIVE_USER_AUTODELETE_YEARS = 0
+    `@override_settings`(INACTIVE_USER_AUTODELETE_YEARS=0)
+    def test_delete_inactive_users_disabled(self):
...
-        # Restore setting
-        settings.INACTIVE_USER_AUTODELETE_YEARS = original_setting
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rodatraden/tests.py` at line 1, Import override_settings from django.test and
use it to wrap the two test methods that currently modify
settings.INACTIVE_USER_AUTODELETE_YEARS (remove the manual assignment/restore).
Specifically, add "from django.test import override_settings" to the imports and
annotate each test method inside the TestCase subclass that mutates
INACTIVE_USER_AUTODELETE_YEARS with
`@override_settings`(INACTIVE_USER_AUTODELETE_YEARS=<desired_value>) so the
setting is automatically restored; remove the lines that set and reset
settings.INACTIVE_USER_AUTODELETE_YEARS within those test methods.

from django.contrib.auth.models import User
from django.utils import timezone
from django.contrib.admin.sites import AdminSite
from django.contrib import messages
from unittest.mock import Mock
from datetime import timedelta
from django.conf import settings

# Create your tests here.
from .admin import CustomUserAdmin, delete_inactive_users


class InactiveUserDeletionTest(TestCase):
def setUp(self):
self.factory = RequestFactory()
self.admin_site = AdminSite()
self.user_admin = CustomUserAdmin(User, self.admin_site)

# Create test users
self.old_user = User.objects.create_user(
username='olduser',
email='old@example.com',
date_joined=timezone.now() - timedelta(days=6*365) # 6 years ago
)
self.old_user.last_login = timezone.now() - timedelta(days=6*365)
self.old_user.save()

self.new_user = User.objects.create_user(
username='newuser',
email='new@example.com',
date_joined=timezone.now() - timedelta(days=1*365) # 1 year ago
)
self.new_user.last_login = timezone.now() - timedelta(days=1*365)
self.new_user.save()

self.never_logged_in_old = User.objects.create_user(
username='neverold',
email='never@example.com',
date_joined=timezone.now() - timedelta(days=6*365) # 6 years ago
)
# last_login remains None

self.staff_user = User.objects.create_user(
username='staff',
email='staff@example.com',
is_staff=True
)

def test_delete_inactive_users(self):
# Set the threshold to 5 years
original_setting = getattr(settings, 'INACTIVE_USER_AUTODELETE_YEARS', 5)
settings.INACTIVE_USER_AUTODELETE_YEARS = 5

request = self.factory.post('/')
request.user = self.staff_user

# Mock message_user
self.user_admin.message_user = Mock()

# Call the action
delete_inactive_users(self.user_admin, request, User.objects.all())

# Check that old users were deleted
self.assertFalse(User.objects.filter(username='olduser').exists())
self.assertFalse(User.objects.filter(username='neverold').exists())

# Check that new user and staff were not deleted
self.assertTrue(User.objects.filter(username='newuser').exists())
self.assertTrue(User.objects.filter(username='staff').exists())

# Restore setting
settings.INACTIVE_USER_AUTODELETE_YEARS = original_setting

def test_delete_inactive_users_disabled(self):
# Set to disabled
original_setting = getattr(settings, 'INACTIVE_USER_AUTODELETE_YEARS', 5)
settings.INACTIVE_USER_AUTODELETE_YEARS = 0

request = self.factory.post('/')
request.user = self.staff_user

# Mock message_user
self.user_admin.message_user = Mock()

# Call the action
delete_inactive_users(self.user_admin, request, User.objects.all())

# No users should be deleted
self.assertTrue(User.objects.filter(username='olduser').exists())
self.assertTrue(User.objects.filter(username='neverold').exists())
self.assertTrue(User.objects.filter(username='newuser').exists())
self.assertTrue(User.objects.filter(username='staff').exists())

# Restore setting
settings.INACTIVE_USER_AUTODELETE_YEARS = original_setting
9 changes: 8 additions & 1 deletion tf/settings-template.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,4 +169,11 @@
LOGIN_URL = '/anvandare/login/' # Url for login

# Crispy forms
CRISPY_TEMPLATE_PACK = 'bootstrap5'
CRISPY_TEMPLATE_PACK = 'bootstrap5'

# Inactive user auto-deletion
# Set to number of years after which inactive users are deleted.
# Users are considered inactive if their last_login is older than this threshold,
# or if they never logged in, their date_joined is older than this threshold.
# Set to 0 to disable auto-deletion.
INACTIVE_USER_AUTODELETE_YEARS = 5
Loading