Fix that state deletion bug. Thanks @basraah

Also sets state to guest on inactivation, and reassesses on reactivation. With a unit test to boot!

authentication/managers.py

@@ -1,5 +1,6 @@
 from __future__ import unicode_literals
 from django.db.models import Manager, QuerySet, Q
+from django.db import transaction
 from eveonline.managers import EveManager
 from eveonline.models import EveCharacter
 import logging
@@ -36,6 +37,21 @@ class StateQuerySet(QuerySet):
         else:
             return self.none()
 
+    def get_for_user(self, user):
+        states = self.available_to_user(user)
+        if states.exists():
+            return states[0]
+        else:
+            from authentication.models import get_guest_state
+            return get_guest_state()
+
+    def delete(self):
+        with transaction.atomic():
+            for state in self:
+                for profile in state.userprofile_set.all():
+                    profile.assign_state(state=self.model.objects.exclude(pk=state.pk).get_for_user(profile.user))
+        super(StateQuerySet, self).delete()
+
 
 class StateManager(Manager):
     def get_queryset(self):
@@ -56,9 +72,4 @@ class StateManager(Manager):
             return get_guest_state()
 
     def get_for_user(self, user):
-        states = self.get_queryset().available_to_user(user)
-        if states.exists():
-            return states[0]
-        else:
-            from authentication.models import get_guest_state
-            return get_guest_state()
+        return self.get_queryset().get_for_user(user)

authentication/migrations/0015_user_profiles.py

@@ -182,12 +182,8 @@ class Migration(migrations.Migration):
             fields=[
                 ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
                 ('owner_hash', models.CharField(max_length=28, unique=True)),
-                ('character',
-                 models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='character_ownership',
-                                      to='eveonline.EveCharacter')),
-                ('user',
-                 models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='character_ownerships',
-                                   to=settings.AUTH_USER_MODEL)),
+                ('character', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='character_ownership', to='eveonline.EveCharacter')),
+                ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='character_ownerships', to=settings.AUTH_USER_MODEL)),
             ],
             options={
                 'default_permissions': ('change', 'delete'),
@@ -199,18 +195,15 @@ class Migration(migrations.Migration):
             fields=[
                 ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
                 ('name', models.CharField(max_length=20, unique=True)),
-                ('priority', models.IntegerField(unique=True,
-                                                 help_text="Users get assigned the state with the highest priority available to them.")),
-                ('public', models.BooleanField(default=False, help_text="Make this state available to any character.")),
-                ('member_alliances', models.ManyToManyField(blank=True, to='eveonline.EveAllianceInfo',
-                                                            help_text="Alliances to whose members this state is available.")),
-                ('member_characters', models.ManyToManyField(blank=True, to='eveonline.EveCharacter',
-                                                             help_text="Characters to which this state is available.")),
-                ('member_corporations', models.ManyToManyField(blank=True, to='eveonline.EveCorporationInfo',
-                                                               help_text="Corporations to whose members this state is available.")),
+                ('priority', models.IntegerField(help_text='Users get assigned the state with the highest priority available to them.', unique=True)),
+                ('public', models.BooleanField(default=False, help_text='Make this state available to any character.')),
+                ('member_alliances', models.ManyToManyField(blank=True, help_text='Alliances to whose members this state is available.', to='eveonline.EveAllianceInfo')),
+                ('member_characters', models.ManyToManyField(blank=True, help_text='Characters to which this state is available.', to='eveonline.EveCharacter')),
+                ('member_corporations', models.ManyToManyField(blank=True, help_text='Corporations to whose members this state is available.', to='eveonline.EveCorporationInfo')),
                 ('permissions', models.ManyToManyField(blank=True, to='auth.Permission')),
             ],
             options={
+                'default_permissions': ('change',),
                 'ordering': ['-priority'],
             },
         ),
@@ -218,14 +211,9 @@ class Migration(migrations.Migration):
             name='UserProfile',
             fields=[
                 ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
-                ('main_character',
-                 models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL,
-                                      to='eveonline.EveCharacter')),
-                ('state', models.ForeignKey(on_delete=models.SET(authentication.models.get_guest_state),
-                                            to='authentication.State',
-                                            default=authentication.models.get_guest_state_pk)),
-                ('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='profile',
-                                              to=settings.AUTH_USER_MODEL)),
+                ('main_character', models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='eveonline.EveCharacter')),
+                ('state', models.ForeignKey(default=authentication.models.get_guest_state_pk, on_delete=django.db.models.deletion.SET_DEFAULT, to='authentication.State')),
+                ('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='profile', to=settings.AUTH_USER_MODEL)),
             ],
             options={
                 'default_permissions': ('change',),
@@ -244,4 +232,32 @@ class Migration(migrations.Migration):
             name='AuthServicesInfo',
         ),
         migrations.RunPython(disable_passwords, migrations.RunPython.noop),
+        migrations.CreateModel(
+            name='ProxyPermission',
+            fields=[
+            ],
+            options={
+                'proxy': True,
+                'verbose_name': 'permission',
+                'verbose_name_plural': 'permissions',
+            },
+            bases=('auth.permission',),
+            managers=[
+                ('objects', django.contrib.auth.models.PermissionManager()),
+            ],
+        ),
+        migrations.CreateModel(
+            name='ProxyUser',
+            fields=[
+            ],
+            options={
+                'proxy': True,
+                'verbose_name': 'user',
+                'verbose_name_plural': 'users',
+            },
+            bases=('auth.user',),
+            managers=[
+                ('objects', django.contrib.auth.models.UserManager()),
+            ],
+        ),
     ]

authentication/models.py

@@ -1,6 +1,6 @@
 from __future__ import unicode_literals
 from django.utils.encoding import python_2_unicode_compatible
-from django.db import models
+from django.db import models, transaction
 from django.contrib.auth.models import User, Permission
 from authentication.managers import CharacterOwnershipManager, StateManager
 from eveonline.models import EveCharacter, EveCorporationInfo, EveAllianceInfo
@@ -30,7 +30,6 @@ class State(models.Model):
 
     class Meta:
         ordering = ['-priority']
-        default_permissions = ('change',)
 
     def __str__(self):
         return self.name
@@ -41,6 +40,12 @@ class State(models.Model):
     def available_to_user(self, user):
         return self in State.objects.available_to_user(user)
 
+    def delete(self, **kwargs):
+        with transaction.atomic():
+            for profile in self.userprofile_set.all():
+                profile.assign_state(state=State.objects.exclude(pk=self.pk).get_for_user(profile.user))
+        super(State, self).delete(**kwargs)
+
 
 def get_guest_state():
     try:
@@ -60,9 +65,10 @@ class UserProfile(models.Model):
 
     user = models.OneToOneField(User, related_name='profile', on_delete=models.CASCADE)
     main_character = models.OneToOneField(EveCharacter, blank=True, null=True, on_delete=models.SET_NULL)
-    state = models.ForeignKey(State, on_delete=models.SET(get_guest_state), default=get_guest_state_pk)
+    state = models.ForeignKey(State, on_delete=models.SET_DEFAULT, default=get_guest_state_pk)
 
-    def assign_state(self, commit=True):
+    def assign_state(self, state=None, commit=True):
+        if not state:
             state = State.objects.get_for_user(self.user)
         if self.state != state:
             self.state = state

authentication/signals.py

@@ -1,5 +1,5 @@
 from __future__ import unicode_literals
-from django.db.models.signals import post_save, pre_delete, m2m_changed
+from django.db.models.signals import post_save, pre_delete, m2m_changed, pre_save
 from django.db.models import Q
 from django.dispatch import receiver, Signal
 from django.contrib.auth.models import User
@@ -52,18 +52,6 @@ def state_saved(sender, instance, *args, **kwargs):
     trigger_state_check(instance)
 
 
-@receiver(pre_delete, sender=State)
-def state_deleted(sender, instance, *args, **kwargs):
-    for profile in instance.userprofile_set.all():
-        available = State.objects.exclude(pk=instance.pk).available_to_user(profile.user)
-        if available:
-            profile.state = available[0]
-        else:
-            profile.state = get_guest_state()
-        profile.save(update_fields=['state'])
-        state_changed.send(sender=State, user=profile.user, state=profile.state)
-
-
 # Is there a smarter way to intercept pre_save with a diff main_character or state?
 @receiver(post_save, sender=UserProfile)
 def reassess_on_profile_save(sender, instance, created, *args, **kwargs):
@@ -119,12 +107,17 @@ def validate_main_character_token(sender, instance, *args, **kwargs):
             profile.save()
 
 
-@receiver(post_save, sender=User)
-def assign_state_on_reactivate(sender, instance, *args, **kwargs):
-    # There's no easy way to trigger an action upon saving from pre_save signal
-    # If we're saving a user and that user is in the Guest state, assume is_active was just set to True and assign state
-    if instance.is_active and instance.profile.state == get_guest_state():
+@receiver(pre_save, sender=User)
+def assign_state_on_active_change(sender, instance, *args, **kwargs):
+    # set to guest state if inactive, assign proper state if reactivated
+    if instance.pk:
+        old_instance = User.objects.get(pk=instance.pk)
+        if old_instance.is_active != instance.is_active:
+            if instance.is_active:
                 instance.profile.assign_state()
+            else:
+                instance.profile.state = get_guest_state()
+                instance.profile.save(update_fields=['state'])
 
 
 @receiver(post_save, sender=EveCharacter)

authentication/tests.py

@@ -239,6 +239,7 @@ class StateTestCase(TestCase):
         self._refresh_user()
         self.assertEquals(higher_state, self.user.profile.state)
         higher_state.delete()
+        self.assertFalse(State.objects.filter(name='Higher State').count())
         self._refresh_user()
         self.assertEquals(self.member_state, self.user.profile.state)
 
@@ -258,3 +259,14 @@ class StateTestCase(TestCase):
         higher_state.save()
         self._refresh_user()
         self.assertEquals(self.member_state, self.user.profile.state)
+
+    def test_state_assignment_on_active_changed(self):
+        self.member_state.member_characters.add(self.test_character)
+        self.user.is_active = False
+        self.user.save()
+        self._refresh_user()
+        self.assertEquals(self.user.profile.state, self.guest_state)
+        self.user.is_active = True
+        self.user.save()
+        self._refresh_user()
+        self.assertEquals(self.user.profile.state, self.member_state)