From dea78ea189bccb2404f018659f2592997d558d3a Mon Sep 17 00:00:00 2001 From: Ramon Wenger Date: Tue, 10 Oct 2023 14:53:53 +0200 Subject: [PATCH] Add some changes for resolving an obscure bug with user settings --- server/users/admin.py | 20 ++++++++- .../commands/validate_user_settings.py | 15 +++++++ server/users/models.py | 5 ++- server/users/schema/mutations.py | 10 ++--- server/users/tests/test_join_class.py | 2 +- server/users/validate_user_settings.py | 42 +++++++++++++++++++ 6 files changed, 85 insertions(+), 9 deletions(-) create mode 100644 server/users/management/commands/validate_user_settings.py create mode 100644 server/users/validate_user_settings.py diff --git a/server/users/admin.py b/server/users/admin.py index 00fa9251..ba4a38b5 100644 --- a/server/users/admin.py +++ b/server/users/admin.py @@ -2,13 +2,18 @@ from django.contrib import admin from django.contrib.auth.admin import UserAdmin from users.forms import CustomUserCreationForm, CustomUserChangeForm -from .models import User, SchoolClass, Role, UserRole, UserSetting, License, UserData, Team +from .models import User, SchoolClass, Role, UserRole, UserSetting, License, UserData, Team, SchoolClassMember class SchoolClassInline(admin.TabularInline): model = SchoolClass.users.through extra = 1 + # Prevents change permission to reduce loading time on school class page + def has_change_permission(self, request, obj=None): + return False + + class RoleInline(admin.TabularInline): model = UserRole @@ -18,6 +23,8 @@ class RoleInline(admin.TabularInline): @admin.register(SchoolClass) class SchoolClassAdmin(admin.ModelAdmin): list_display = ('name', 'code', 'user_list', 'is_deleted') + ordering = ('name',) + readonly_fields = ['users',] inlines = [ SchoolClassInline @@ -49,7 +56,7 @@ class CustomUserAdmin(UserAdmin): model = User list_display = ('username', 'first_name', 'last_name', 'school_classes_list', 'is_superuser', 'hep_id') list_filter = ('school_classes', 'is_superuser') - ordering = ['pk'] + ordering = ['username', 'pk'] search_fields = ('username', 'first_name', 'last_name') autocomplete_fields = ('team',) @@ -74,6 +81,8 @@ class UserSettingAdmin(admin.ModelAdmin): list_display = ('user', 'selected_class') raw_id_fields = ('user', 'selected_class') + search_fields = ('user__email', ) + @admin.register(License) class LicenseAdmin(admin.ModelAdmin): @@ -97,3 +106,10 @@ class TeamAdmin(admin.ModelAdmin): def member_list(self, obj): return ', '.join([member.full_name for member in obj.members.all()]) + + +@admin.register(SchoolClassMember) +class SchoolclassMemberAdmin(admin.ModelAdmin): + list_display = ('user', 'school_class', 'active') + list_filter = ('school_class', 'active') + search_fields = ('user__email','school_class__name') diff --git a/server/users/management/commands/validate_user_settings.py b/server/users/management/commands/validate_user_settings.py new file mode 100644 index 00000000..a84890f4 --- /dev/null +++ b/server/users/management/commands/validate_user_settings.py @@ -0,0 +1,15 @@ + +from django.core.management.base import BaseCommand + +from users.validate_user_settings import validate_user_settings + + +class Command(BaseCommand): + + def add_arguments(self, parser): + parser.add_argument('--dry-run', action='store_true', dest='dry_run', default=True, help='Make dry-run') + + def handle(self, *args, **options): + dry_run = options.get('dry_run') + + validate_user_settings(dry_run=dry_run) diff --git a/server/users/models.py b/server/users/models.py index 5fc79d38..9ce863c2 100644 --- a/server/users/models.py +++ b/server/users/models.py @@ -149,7 +149,7 @@ class User(AbstractUser): return User.LICENSE_EXPIRED class Meta: - ordering = ['pk', ] + ordering = ['username'] class GroupWithCode(models.Model): @@ -384,3 +384,6 @@ class SchoolClassMember(models.Model): user = models.ForeignKey(User, on_delete=models.CASCADE) school_class = models.ForeignKey(SchoolClass, on_delete=models.CASCADE) active = models.BooleanField(default=True) + + class Meta: + ordering = ['user__username'] diff --git a/server/users/schema/mutations.py b/server/users/schema/mutations.py index 02e0af29..134f63ac 100644 --- a/server/users/schema/mutations.py +++ b/server/users/schema/mutations.py @@ -140,17 +140,17 @@ class JoinClass(relay.ClientIDMutation): school_class = SchoolClass.objects.get(Q(code__iexact=code)) if user not in list(school_class.users.all()): - SchoolClassMember.objects.create(user=user, school_class=school_class) + SchoolClassMember.objects.get_or_create(user=user, school_class=school_class) user.set_selected_class(school_class) else: raise CodeNotFoundException( - "[CAJ] Schüler ist bereits in Klasse" + f"[CAJ] Schüler ist bereits in Klasse" ) # CAJ = Class Already Joined return cls(success=True, school_class=school_class) except SchoolClass.DoesNotExist: raise CodeNotFoundException( - "[CNV] Code ist nicht gültig" + f"[CNV] Code ist nicht gültig '{code}'" ) # CNV = Code Not Valid @@ -219,7 +219,7 @@ class CreateSchoolClass(TeacherOnlyMutation): try: school_class = SchoolClass.objects.create(name=name) - SchoolClassMember.objects.create(school_class=school_class, user=user) + SchoolClassMember.objects.get_or_create(school_class=school_class, user=user) user.set_selected_class(school_class) return cls(result=school_class) except IntegrityError: @@ -290,7 +290,7 @@ class JoinTeam(TeacherOnlyMutation): return cls(success=True, team=team) except Team.DoesNotExist: raise CodeNotFoundException( - "[CNV] Code ist nicht gültig" + f"[CNV] Code ist nicht gültig '{code}'" ) # CNV = Code Not Valid diff --git a/server/users/tests/test_join_class.py b/server/users/tests/test_join_class.py index 6bfe0db7..5820f97f 100644 --- a/server/users/tests/test_join_class.py +++ b/server/users/tests/test_join_class.py @@ -68,5 +68,5 @@ class JoinSchoolClassTest(TestCase): } }, context=self.context) self.assertIsNotNone(executed['errors']) - self.assertEqual(executed['errors'][0]['message'], '[CNV] Code ist nicht gültig') + self.assertEqual(executed['errors'][0]['message'], "[CNV] Code ist nicht gültig '1234'") self.assertEqual(self.user.school_classes.count(), 1) diff --git a/server/users/validate_user_settings.py b/server/users/validate_user_settings.py new file mode 100644 index 00000000..11085852 --- /dev/null +++ b/server/users/validate_user_settings.py @@ -0,0 +1,42 @@ +from .models import SchoolClassMember, UserSetting, User + + +# This code was made for a support case where some users had invalid user settings. +def validate_user_setting(user_setting, dry_run=True): + + try: + SchoolClassMember.objects.get(user=user_setting.user, school_class=user_setting.selected_class) + + # Test for the invalid user settings and delete them (user settings are gereated automatically) + except SchoolClassMember.DoesNotExist as e: + print(e) + print(f"invalid user setting: {user_setting.user.email} {user_setting.user.selected_class}") + if not dry_run: + user_id = user_setting.user.id + try: + del user_setting.user.selected_class + except AttributeError: + pass + + user_setting.delete() + user = User.objects.get(id=user_id) + + print(f"New user setting: {user.email} {user.selected_class}\n") + + # Errors with SchoolClassMember.MultipleObjectsReturned + # A bug created duplicates in the database. This code deletes the duplicates. + except SchoolClassMember.MultipleObjectsReturned as e: + print(e) + count = 0 + for member in SchoolClassMember.objects.filter(user=user_setting.user): + print(f"Multiple SchoolClassMembers for user {count}: {member.id} {member.user.email}: {member.school_class.name} {member.active}") + if count >=1: + print("delete") + if not dry_run: + member.delete() + count += 1 + print("\n") + +def validate_user_settings(dry_run=True): + for user_setting in UserSetting.objects.all(): + validate_user_setting(user_setting, dry_run=dry_run)