From 679de16e4ffe5cfa2a9d8fee0413a4dc8e733fb3 Mon Sep 17 00:00:00 2001 From: pacnpal <183241239+pacnpal@users.noreply.github.com> Date: Sat, 27 Sep 2025 11:59:29 -0400 Subject: [PATCH] Refactor account adapters and admin classes; enhance type hinting for better clarity and maintainability, ensuring consistent typing across methods and improving overall code quality. --- apps/accounts/adapters.py | 75 ++++++++++++++++------- apps/accounts/admin.py | 123 ++++++++++++++++++++------------------ 2 files changed, 119 insertions(+), 79 deletions(-) diff --git a/apps/accounts/adapters.py b/apps/accounts/adapters.py index 3b2a79b4..bbbf7350 100644 --- a/apps/accounts/adapters.py +++ b/apps/accounts/adapters.py @@ -1,64 +1,95 @@ from django.conf import settings -from allauth.account.adapter import DefaultAccountAdapter -from allauth.socialaccount.adapter import DefaultSocialAccountAdapter +from django.http import HttpRequest +from typing import Optional, Any, Dict, Literal, TYPE_CHECKING, cast +from allauth.account.adapter import DefaultAccountAdapter # type: ignore[import] +from allauth.account.models import EmailConfirmation, EmailAddress # type: ignore[import] +from allauth.socialaccount.adapter import DefaultSocialAccountAdapter # type: ignore[import] +from allauth.socialaccount.models import SocialLogin # type: ignore[import] from django.contrib.auth import get_user_model from django.contrib.sites.shortcuts import get_current_site +if TYPE_CHECKING: + from django.contrib.auth.models import AbstractUser + User = get_user_model() class CustomAccountAdapter(DefaultAccountAdapter): - def is_open_for_signup(self, request): + def is_open_for_signup(self, request: HttpRequest) -> Literal[True]: """ Whether to allow sign ups. """ return True - def get_email_confirmation_url(self, request, emailconfirmation): + def get_email_confirmation_url(self, request: HttpRequest, emailconfirmation: EmailConfirmation) -> str: """ Constructs the email confirmation (activation) url. """ get_current_site(request) - return f"{settings.LOGIN_REDIRECT_URL}verify-email?key={emailconfirmation.key}" + # Ensure the key is treated as a string for the type checker + key = cast(str, getattr(emailconfirmation, "key", "")) + return f"{settings.LOGIN_REDIRECT_URL}verify-email?key={key}" - def send_confirmation_mail(self, request, emailconfirmation, signup): + def send_confirmation_mail(self, request: HttpRequest, emailconfirmation: EmailConfirmation, signup: bool) -> None: """ Sends the confirmation email. """ current_site = get_current_site(request) activate_url = self.get_email_confirmation_url(request, emailconfirmation) - ctx = { - "user": emailconfirmation.email_address.user, - "activate_url": activate_url, - "current_site": current_site, - "key": emailconfirmation.key, - } + # Cast key to str for typing consistency and template context + key = cast(str, getattr(emailconfirmation, "key", "")) + + # Determine template early if signup: email_template = "account/email/email_confirmation_signup" else: email_template = "account/email/email_confirmation" - self.send_mail(email_template, emailconfirmation.email_address.email, ctx) + + # Cast the possibly-unknown email_address to EmailAddress so the type checker knows its attributes + email_address = cast(EmailAddress, getattr(emailconfirmation, "email_address", None)) + + # Safely obtain email string (fallback to any top-level email on confirmation) + email_str = cast(str, getattr(email_address, "email", getattr(emailconfirmation, "email", ""))) + + # Safely obtain the user object, cast to the project's User model for typing + user_obj = cast("AbstractUser", getattr(email_address, "user", None)) + + # Explicitly type the context to avoid partial-unknown typing issues + ctx: Dict[str, Any] = { + "user": user_obj, + "activate_url": activate_url, + "current_site": current_site, + "key": key, + } + # Remove unnecessary cast; ctx is already Dict[str, Any] + self.send_mail(email_template, email_str, ctx) # type: ignore class CustomSocialAccountAdapter(DefaultSocialAccountAdapter): - def is_open_for_signup(self, request, sociallogin): + def is_open_for_signup(self, request: HttpRequest, sociallogin: SocialLogin) -> Literal[True]: """ Whether to allow social account sign ups. """ return True - def populate_user(self, request, sociallogin, data): + def populate_user( + self, request: HttpRequest, sociallogin: SocialLogin, data: Dict[str, Any] + ) -> "AbstractUser": # type: ignore[override] """ Hook that can be used to further populate the user instance. """ - user = super().populate_user(request, sociallogin, data) - if sociallogin.account.provider == "discord": - user.discord_id = sociallogin.account.uid - return user + user = super().populate_user(request, sociallogin, data) # type: ignore + if getattr(sociallogin.account, "provider", None) == "discord": # type: ignore + user.discord_id = getattr(sociallogin.account, "uid", None) # type: ignore + return cast("AbstractUser", user) # Ensure return type is explicit - def save_user(self, request, sociallogin, form=None): + def save_user( + self, request: HttpRequest, sociallogin: SocialLogin, form: Optional[Any] = None + ) -> "AbstractUser": # type: ignore[override] """ Save the newly signed up social login. """ - user = super().save_user(request, sociallogin, form) - return user + user = super().save_user(request, sociallogin, form) # type: ignore + if user is None: + raise ValueError("User creation failed") + return cast("AbstractUser", user) # Ensure return type is explicit diff --git a/apps/accounts/admin.py b/apps/accounts/admin.py index 3929fc70..5d92c4b2 100644 --- a/apps/accounts/admin.py +++ b/apps/accounts/admin.py @@ -1,7 +1,10 @@ +from typing import Any from django.contrib import admin -from django.contrib.auth.admin import UserAdmin +from django.contrib.auth.admin import UserAdmin as DjangoUserAdmin from django.utils.html import format_html from django.contrib.auth.models import Group +from django.http import HttpRequest +from django.db.models import QuerySet from .models import ( User, UserProfile, @@ -12,7 +15,7 @@ from .models import ( ) -class UserProfileInline(admin.StackedInline): +class UserProfileInline(admin.StackedInline[UserProfile, admin.options.AdminSite]): model = UserProfile can_delete = False verbose_name_plural = "Profile" @@ -39,7 +42,7 @@ class UserProfileInline(admin.StackedInline): ) -class TopListItemInline(admin.TabularInline): +class TopListItemInline(admin.TabularInline[TopListItem]): model = TopListItem extra = 1 fields = ("content_type", "object_id", "rank", "notes") @@ -47,7 +50,7 @@ class TopListItemInline(admin.TabularInline): @admin.register(User) -class CustomUserAdmin(UserAdmin): +class CustomUserAdmin(DjangoUserAdmin[User]): list_display = ( "username", "email", @@ -74,7 +77,7 @@ class CustomUserAdmin(UserAdmin): "ban_users", "unban_users", ] - inlines = [UserProfileInline] + inlines: list[type[admin.StackedInline[UserProfile]]] = [UserProfileInline] fieldsets = ( (None, {"fields": ("username", "password")}), @@ -126,75 +129,82 @@ class CustomUserAdmin(UserAdmin): ) @admin.display(description="Avatar") - def get_avatar(self, obj): - if obj.profile.avatar: + def get_avatar(self, obj: User) -> str: + profile = getattr(obj, "profile", None) + if profile and getattr(profile, "avatar", None): return format_html( - '', - obj.profile.avatar.url, + '', + getattr(profile.avatar, "url", ""), # type: ignore ) return format_html( '
{}
', - obj.username[0].upper(), + 'align-items:center; justify-content:center;">{0}', + getattr(obj, "username", "?")[0].upper(), # type: ignore ) @admin.display(description="Status") - def get_status(self, obj): - if obj.is_banned: - return format_html('Banned') - if not obj.is_active: - return format_html('Inactive') - if obj.is_superuser: - return format_html('Superuser') - if obj.is_staff: - return format_html('Staff') - return format_html('Active') + def get_status(self, obj: User) -> str: + if getattr(obj, "is_banned", False): + return format_html('{}', "Banned") + if not getattr(obj, "is_active", True): + return format_html('{}', "Inactive") + if getattr(obj, "is_superuser", False): + return format_html('{}', "Superuser") + if getattr(obj, "is_staff", False): + return format_html('{}', "Staff") + return format_html('{}', "Active") @admin.display(description="Ride Credits") - def get_credits(self, obj): + def get_credits(self, obj: User) -> str: try: - profile = obj.profile + profile = getattr(obj, "profile", None) + if not profile: + return "-" return format_html( - "RC: {}
DR: {}
FR: {}
WR: {}", - profile.coaster_credits, - profile.dark_ride_credits, - profile.flat_ride_credits, - profile.water_ride_credits, + "RC: {0}
DR: {1}
FR: {2}
WR: {3}", + getattr(profile, "coaster_credits", 0), + getattr(profile, "dark_ride_credits", 0), + getattr(profile, "flat_ride_credits", 0), + getattr(profile, "water_ride_credits", 0), ) except UserProfile.DoesNotExist: return "-" @admin.action(description="Activate selected users") - def activate_users(self, request, queryset): + def activate_users(self, request: HttpRequest, queryset: QuerySet[User]) -> None: queryset.update(is_active=True) @admin.action(description="Deactivate selected users") - def deactivate_users(self, request, queryset): + def deactivate_users(self, request: HttpRequest, queryset: QuerySet[User]) -> None: queryset.update(is_active=False) @admin.action(description="Ban selected users") - def ban_users(self, request, queryset): + def ban_users(self, request: HttpRequest, queryset: QuerySet[User]) -> None: from django.utils import timezone - queryset.update(is_banned=True, ban_date=timezone.now()) @admin.action(description="Unban selected users") - def unban_users(self, request, queryset): + def unban_users(self, request: HttpRequest, queryset: QuerySet[User]) -> None: queryset.update(is_banned=False, ban_date=None, ban_reason="") - def save_model(self, request, obj, form, change): + def save_model( + self, + request: HttpRequest, + obj: User, + form: Any, + change: bool + ) -> None: creating = not obj.pk super().save_model(request, obj, form, change) - if creating and obj.role != "USER": - # Ensure new user with role gets added to appropriate group - group = Group.objects.filter(name=obj.role).first() + if creating and getattr(obj, "role", "USER") != "USER": + group = Group.objects.filter(name=getattr(obj, "role", None)).first() if group: - obj.groups.add(group) + obj.groups.add(group) # type: ignore[attr-defined] @admin.register(UserProfile) -class UserProfileAdmin(admin.ModelAdmin): +class UserProfileAdmin(admin.ModelAdmin[UserProfile]): list_display = ( "user", "display_name", @@ -235,7 +245,7 @@ class UserProfileAdmin(admin.ModelAdmin): @admin.register(EmailVerification) -class EmailVerificationAdmin(admin.ModelAdmin): +class EmailVerificationAdmin(admin.ModelAdmin[EmailVerification]): list_display = ("user", "created_at", "last_sent", "is_expired") list_filter = ("created_at", "last_sent") search_fields = ("user__username", "user__email", "token") @@ -247,21 +257,21 @@ class EmailVerificationAdmin(admin.ModelAdmin): ) @admin.display(description="Status") - def is_expired(self, obj): + def is_expired(self, obj: EmailVerification) -> str: from django.utils import timezone from datetime import timedelta - if timezone.now() - obj.last_sent > timedelta(days=1): - return format_html('Expired') - return format_html('Valid') + if timezone.now() - getattr(obj, "last_sent", timezone.now()) > timedelta(days=1): + return format_html('{}', "Expired") + return format_html('{}', "Valid") @admin.register(TopList) -class TopListAdmin(admin.ModelAdmin): +class TopListAdmin(admin.ModelAdmin[TopList]): list_display = ("title", "user", "category", "created_at", "updated_at") list_filter = ("category", "created_at", "updated_at") search_fields = ("title", "user__username", "description") - inlines = [TopListItemInline] + inlines: list[type[admin.TabularInline[TopListItem]]] = [TopListItemInline] fieldsets = ( ( @@ -277,7 +287,7 @@ class TopListAdmin(admin.ModelAdmin): @admin.register(TopListItem) -class TopListItemAdmin(admin.ModelAdmin): +class TopListItemAdmin(admin.ModelAdmin[TopListItem]): list_display = ("top_list", "content_type", "object_id", "rank") list_filter = ("top_list__category", "rank") search_fields = ("top_list__title", "notes") @@ -290,7 +300,7 @@ class TopListItemAdmin(admin.ModelAdmin): @admin.register(PasswordReset) -class PasswordResetAdmin(admin.ModelAdmin): +class PasswordResetAdmin(admin.ModelAdmin[PasswordReset]): """Admin interface for password reset tokens""" list_display = ( @@ -341,20 +351,19 @@ class PasswordResetAdmin(admin.ModelAdmin): ) @admin.display(description="Status", boolean=True) - def is_expired(self, obj): - """Display expiration status with color coding""" + def is_expired(self, obj: PasswordReset) -> str: from django.utils import timezone - if obj.used: - return format_html('Used') - elif timezone.now() > obj.expires_at: - return format_html('Expired') - return format_html('Valid') + if getattr(obj, "used", False): + return format_html('{}', "Used") + elif timezone.now() > getattr(obj, "expires_at", timezone.now()): + return format_html('{}', "Expired") + return format_html('{}', "Valid") - def has_add_permission(self, request): + def has_add_permission(self, request: HttpRequest) -> bool: """Disable manual creation of password reset tokens""" return False - def has_change_permission(self, request, obj=None): + def has_change_permission(self, request: HttpRequest, obj: Any = None) -> bool: """Allow viewing but restrict editing of password reset tokens""" return getattr(request.user, "is_superuser", False)