From bf6f3fdec231555fb06b7f681d23e8ebeb0cbcd2 Mon Sep 17 00:00:00 2001
From: Jean-Romain Garnier <jean-romain.garnier@supelec.fr>
Date: Wed, 15 Apr 2020 19:04:14 +0000
Subject: [PATCH] Make call to remove_user_room cleaner

---
 re2o/utils.py  |  8 +++++---
 users/forms.py | 38 +++++++++++++++-----------------------
 2 files changed, 20 insertions(+), 26 deletions(-)

diff --git a/re2o/utils.py b/re2o/utils.py
index 348e5c55..7c49ff0a 100644
--- a/re2o/utils.py
+++ b/re2o/utils.py
@@ -203,11 +203,13 @@ def all_active_assigned_interfaces_count():
     return all_active_interfaces_count().filter(ipv4__isnull=False)
 
 
-def remove_user_room(room):
+def remove_user_room(room, force=True):
     """ Déménage de force l'ancien locataire de la chambre """
     try:
         user = Adherent.objects.get(room=room)
     except Adherent.DoesNotExist:
         return
-    user.room = None
-    user.save()
+
+    if force or not user.has_access():
+        user.room = None
+        user.save()
diff --git a/users/forms.py b/users/forms.py
index 322c715e..a020a427 100644
--- a/users/forms.py
+++ b/users/forms.py
@@ -351,29 +351,6 @@ class AdherentForm(FormRevMixin, FieldPermissionFormMixin, ModelForm):
         label=_("Force the move?"), initial=False, required=False
     )
 
-    def clean(self):
-        # Handle case where regular users can force move
-        can_force_move = OptionalUser.get_cached_value("self_force_move_disabled_user_room")
-        if not can_force_move:
-            return super(AdherentForm, self).clean()
-
-        # Ensure the user entered a proper room
-        room = self.cleaned_data.get("room")
-        if not room:
-            return super(AdherentForm, self).clean()
-
-        try:
-            # If a user already is register for this room
-            # but their connection has expired, allow force move
-            user = Adherent.objects.get(room=room)
-            if user and not user.has_access():
-                remove_user_room(room)
-        except Adherent.DoesNotExist:
-            pass
-
-        # Run standard clean process
-        return super(AdherentForm, self).clean()
-
     def clean_email(self):
         if not OptionalUser.objects.first().local_email_domain in self.cleaned_data.get(
             "email"
@@ -402,6 +379,21 @@ class AdherentForm(FormRevMixin, FieldPermissionFormMixin, ModelForm):
             remove_user_room(room)
         return
 
+    def clean_room(self):
+        """On supprime l'ancien user de la chambre si l'option est activée,
+        et que l'ancien user a une connexion désactivée"""
+        # Handle case where regular users can force move
+        room = self.cleaned_data.get("room")
+        can_force_move = OptionalUser.get_cached_value("self_force_move_disabled_user_room")
+        if not can_force_move or not room:
+            return room
+
+        # Remove the previous user's room, if allowed and necessary
+        remove_user_room(room, force=False)
+
+        # Run standard clean process
+        return room
+
 
 class AdherentCreationForm(AdherentForm):
     """Formulaire de création d'un user.