diff --git a/users/__init__.py b/users/__init__.py index df6e4256..b661a850 100644 --- a/users/__init__.py +++ b/users/__init__.py @@ -20,5 +20,12 @@ # You should have received a copy of the GNU General Public License along # with this program; if not, write to the Free Software Foundation, Inc., # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +"""users +The app managing everything related to the users such as personal +informations or the right groups. +This is probably the most central app. It is strongly linked with +all the other because a user has devices (machines), a cotisation +(cotisations), a room (topologie) +""" from .acl import * diff --git a/users/admin.py b/users/admin.py index 727b129c..a902d2e2 100644 --- a/users/admin.py +++ b/users/admin.py @@ -132,7 +132,7 @@ class UserAdmin(VersionAdmin, BaseUserAdmin): ) # Need to reset the settings from BaseUserAdmin # They are using fields we don't use like 'is_staff' - list_filter=() + list_filter = () fieldsets = ( (None, {'fields': ('pseudo', 'password')}), ( diff --git a/users/forms.py b/users/forms.py index a43db749..0a17df8b 100644 --- a/users/forms.py +++ b/users/forms.py @@ -41,6 +41,10 @@ from django.utils import timezone from django.contrib.auth.models import Group, Permission from preferences.models import OptionalUser +from re2o.utils import remove_user_room +from re2o.mixins import FormRevMixin +from re2o.field_permissions import FieldPermissionFormMixin + from .models import ( User, ServiceUser, @@ -52,9 +56,6 @@ from .models import ( Adherent, Club ) -from re2o.utils import remove_user_room -from re2o.mixins import FormRevMixin -from re2o.field_permissions import FieldPermissionFormMixin class PassForm(FormRevMixin, FieldPermissionFormMixin, forms.ModelForm): @@ -97,8 +98,8 @@ class PassForm(FormRevMixin, FieldPermissionFormMixin, forms.ModelForm): def clean_selfpasswd(self): """Verifie si il y a lieu que le mdp self est correct""" if not self.instance.check_password( - self.cleaned_data.get("selfpasswd") - ): + self.cleaned_data.get("selfpasswd") + ): raise forms.ValidationError("Le mot de passe actuel est incorrect") return diff --git a/users/models.py b/users/models.py index a0561aed..47203a29 100644 --- a/users/models.py +++ b/users/models.py @@ -187,6 +187,8 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, (2, 'STATE_ARCHIVE'), ) + # TODO : Use only one of auto_uid and get_fresh_user_uid + @staticmethod def auto_uid(): """Renvoie un uid libre""" return get_fresh_user_uid() @@ -278,10 +280,14 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, @cached_property def is_class_club(self): + """ Returns True if the object is a Club (subclassing User) """ + # TODO : change to isinstance (cleaner) return hasattr(self, 'club') @cached_property def is_class_adherent(self): + """ Returns True if the object is a Adherent (subclassing User) """ + # TODO : change to isinstance (cleaner) return hasattr(self, 'adherent') @property @@ -607,8 +613,8 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, une machine inconnue sur le compte de l'user""" all_interfaces = self.user_interfaces(active=False) if all_interfaces.count() > OptionalMachine.get_cached_value( - 'max_lambdauser_interfaces' - ): + 'max_lambdauser_interfaces' + ): return False, "Maximum de machines enregistrees atteinte" if not nas_type: return False, "Re2o ne sait pas à quel machinetype affecter cette\ @@ -686,8 +692,8 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, num += 1 return composed_pseudo(num) - def can_edit(self, user_request, *args, **kwargs): - """Check if an user can edit an user object. + def can_edit(self, user_request, *_args, **_kwargs): + """Check if a user can edit a user object. :param self: The user which is to be edited. :param user_request: The user who requests to edit self. @@ -722,7 +728,15 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, return False, (u"Vous ne pouvez éditer un autre utilisateur " "que vous même") - def can_change_password(self, user_request, *args, **kwargs): + def can_change_password(self, user_request, *_args, **_kwargs): + """Check if a user can change a user's password + + :param self: The user which is to be edited + :param user_request: The user who request to edit self + :returns: a message and a boolean which is True if self is a club + and user_request one of it's admins, or if user_request is self, + or if user_request has the right to change other's password + """ if self.is_class_club and user_request.is_class_adherent: if (self == user_request or user_request.has_perm('users.change_user_password') or @@ -743,38 +757,65 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, return False, (u"Vous ne pouvez éditer un autre utilisateur " "que vous même") - def check_selfpasswd(self, user_request, *args, **kwargs): + def check_selfpasswd(self, user_request, *_args, **_kwargs): + """ Returns (True, None) if user_request is self, else returns + (False, None) + """ return user_request == self, None @staticmethod - def can_change_state(user_request, *args, **kwargs): + def can_change_state(user_request, *_args, **_kwargs): + """ Check if a user can change a state + + :param user_request: The user who request + :returns: a message and a boolean which is True if the user has + the right to change a state + """ return ( user_request.has_perm('users.change_user_state'), "Droit requis pour changer l'état" ) @staticmethod - def can_change_shell(user_request, *args, **kwargs): + def can_change_shell(user_request, *_args, **_kwargs): + """ Check if a user can change a shell + + :param user_request: The user who request + :returns: a message and a boolean which is True if the user has + the right to change a shell + """ return ( user_request.has_perm('users.change_user_shell'), "Droit requis pour changer le shell" ) @staticmethod - def can_change_force(user_request, *args, **kwargs): + def can_change_force(user_request, *_args, **_kwargs): + """ Check if a user can change a force + + :param user_request: The user who request + :returns: a message and a boolean which is True if the user has + the right to change a force + """ return ( user_request.has_perm('users.change_user_force'), "Droit requis pour forcer le déménagement" ) @staticmethod - def can_change_groups(user_request, *args, **kwargs): + def can_change_groups(user_request, *_args, **_kwargs): + """ Check if a user can change a group + + :param user_request: The user who request + :returns: a message and a boolean which is True if the user has + the right to change a group + """ return ( user_request.has_perm('users.change_user_groups'), "Droit requis pour éditer les groupes de l'user" ) - def can_view(self, user_request, *args, **kwargs): + def can_view(self, user_request, *_args, **_kwargs): """Check if an user can view an user object. :param self: The targeted user. @@ -798,7 +839,8 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, return False, (u"Vous ne pouvez voir un autre utilisateur " "que vous même") - def can_view_all(user_request, *args, **kwargs): + @staticmethod + def can_view_all(user_request, *_args, **_kwargs): """Check if an user can access to the list of every user objects :param user_request: The user who wants to view the list. @@ -810,7 +852,7 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, u"Vous n'avez pas accès à la liste des utilisateurs." ) - def can_delete(self, user_request, *args, **kwargs): + def can_delete(self, user_request, *_args, **_kwargs): """Check if an user can delete an user object. :param self: The user who is to be deleted. @@ -836,6 +878,8 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, class Adherent(User): + """ A class representing a member (it's a user with special + informations) """ PRETTY_NAME = "Adhérents" name = models.CharField(max_length=255) room = models.OneToOneField( @@ -845,15 +889,17 @@ class Adherent(User): null=True ) - def get_instance(adherentid, *args, **kwargs): + @classmethod + def get_instance(cls, adherentid, *_args, **_kwargs): """Try to find an instance of `Adherent` with the given id. :param adherentid: The id of the adherent we are looking for. :return: An adherent. """ - return Adherent.objects.get(pk=adherentid) + return cls.objects.get(pk=adherentid) - def can_create(user_request, *args, **kwargs): + @staticmethod + def can_create(user_request, *_args, **_kwargs): """Check if an user can create an user object. :param user_request: The user who wants to create a user object. @@ -875,6 +921,8 @@ class Adherent(User): class Club(User): + """ A class representing a club (it is considered as a user + with special informations) """ PRETTY_NAME = "Clubs" room = models.ForeignKey( 'topologie.Room', @@ -896,7 +944,8 @@ class Club(User): default=False ) - def can_create(user_request, *args, **kwargs): + @staticmethod + def can_create(user_request, *_args, **_kwargs): """Check if an user can create an user object. :param user_request: The user who wants to create a user object. @@ -914,7 +963,8 @@ class Club(User): u"Vous n'avez pas le droit de créer un club" ) - def can_view_all(user_request, *args, **kwargs): + @staticmethod + def can_view_all(user_request, *_args, **_kwargs): """Check if an user can access to the list of every user objects :param user_request: The user who wants to view the list. @@ -930,22 +980,23 @@ class Club(User): return True, None return False, u"Vous n'avez pas accès à la liste des utilisateurs." - def get_instance(clubid, *args, **kwargs): + @classmethod + def get_instance(cls, clubid, *_args, **_kwargs): """Try to find an instance of `Club` with the given id. :param clubid: The id of the adherent we are looking for. :return: A club. """ - return Club.objects.get(pk=clubid) + return cls.objects.get(pk=clubid) @receiver(post_save, sender=Adherent) @receiver(post_save, sender=Club) @receiver(post_save, sender=User) -def user_post_save(sender, **kwargs): +def user_post_save(_sender, **kwargs): """ Synchronisation post_save : envoie le mail de bienvenue si creation Synchronise le ldap""" - is_created = kwargs['created'] + # is_created = kwargs['created'] user = kwargs['instance'] # TODO : remove if unnecessary # if is_created: @@ -962,7 +1013,7 @@ def user_post_save(sender, **kwargs): @receiver(post_delete, sender=Adherent) @receiver(post_delete, sender=Club) @receiver(post_delete, sender=User) -def user_post_delete(sender, **kwargs): +def user_post_delete(_sender, **kwargs): """Post delete d'un user, on supprime son instance ldap""" user = kwargs['instance'] user.ldap_del() @@ -1005,6 +1056,14 @@ class ServiceUser(RevMixin, AclMixin, AbstractBaseUser): ("view_serviceuser", "Peut voir un objet serviceuser"), ) + def get_full_name(self): + """ Renvoie le nom complet du serviceUser formaté nom/prénom""" + return "ServiceUser <{name}>".format(name=self.pseudo) + + def get_short_name(self): + """ Renvoie seulement le nom""" + return self.pseudo + def ldap_sync(self): """ Synchronisation du ServiceUser dans sa version ldap""" try: @@ -1041,14 +1100,14 @@ class ServiceUser(RevMixin, AclMixin, AbstractBaseUser): @receiver(post_save, sender=ServiceUser) -def service_user_post_save(sender, **kwargs): +def service_user_post_save(_sender, **kwargs): """ Synchronise un service user ldap après modification django""" service_user = kwargs['instance'] service_user.ldap_sync() @receiver(post_delete, sender=ServiceUser) -def service_user_post_delete(sender, **kwargs): +def service_user_post_delete(_sender, **kwargs): """ Supprime un service user ldap après suppression django""" service_user = kwargs['instance'] service_user.ldap_del() @@ -1123,14 +1182,14 @@ class ListRight(RevMixin, AclMixin, Group): @receiver(post_save, sender=ListRight) -def listright_post_save(sender, **kwargs): +def listright_post_save(_sender, **kwargs): """ Synchronise le droit ldap quand il est modifié""" right = kwargs['instance'] right.ldap_sync() @receiver(post_delete, sender=ListRight) -def listright_post_delete(sender, **kwargs): +def listright_post_delete(_sender, **kwargs): """Suppression d'un groupe ldap après suppression coté django""" right = kwargs['instance'] right.ldap_del() @@ -1203,7 +1262,7 @@ class Ban(RevMixin, AclMixin, models.Model): """Ce ban est-il actif?""" return self.date_end > timezone.now() - def can_view(self, user_request, *args, **kwargs): + def can_view(self, user_request, *_args, **_kwargs): """Check if an user can view a Ban object. :param self: The targeted object. @@ -1223,7 +1282,7 @@ class Ban(RevMixin, AclMixin, models.Model): @receiver(post_save, sender=Ban) -def ban_post_save(sender, **kwargs): +def ban_post_save(_sender, **kwargs): """ Regeneration de tous les services après modification d'un ban""" ban = kwargs['instance'] is_created = kwargs['created'] @@ -1240,7 +1299,7 @@ def ban_post_save(sender, **kwargs): @receiver(post_delete, sender=Ban) -def ban_post_delete(sender, **kwargs): +def ban_post_delete(_sender, **kwargs): """ Regen de tous les services après suppression d'un ban""" user = kwargs['instance'].user user.ldap_sync(base=False, access_refresh=True, mac_refresh=False) @@ -1266,9 +1325,10 @@ class Whitelist(RevMixin, AclMixin, models.Model): ) def is_active(self): + """ Is this whitelisting active ? """ return self.date_end > timezone.now() - def can_view(self, user_request, *args, **kwargs): + def can_view(self, user_request, *_args, **_kwargs): """Check if an user can view a Whitelist object. :param self: The targeted object. @@ -1288,7 +1348,7 @@ class Whitelist(RevMixin, AclMixin, models.Model): @receiver(post_save, sender=Whitelist) -def whitelist_post_save(sender, **kwargs): +def whitelist_post_save(_sender, **kwargs): """Après modification d'une whitelist, on synchronise les services et on lui permet d'avoir internet""" whitelist = kwargs['instance'] @@ -1305,7 +1365,7 @@ def whitelist_post_save(sender, **kwargs): @receiver(post_delete, sender=Whitelist) -def whitelist_post_delete(sender, **kwargs): +def whitelist_post_delete(_sender, **kwargs): """Après suppression d'une whitelist, on supprime l'accès internet en forçant la régénration""" user = kwargs['instance'].user diff --git a/users/serializers.py b/users/serializers.py index d2f66abe..be925881 100644 --- a/users/serializers.py +++ b/users/serializers.py @@ -22,11 +22,17 @@ # Maël Kervella +"""users.serializers +Serializers for the User app +""" + from rest_framework import serializers from users.models import Club, Adherent class MailingSerializer(serializers.ModelSerializer): + """ Serializer to build Mailing objects """ + name = serializers.CharField(source='pseudo') class Meta: @@ -35,6 +41,9 @@ class MailingSerializer(serializers.ModelSerializer): class MailingMemberSerializer(serializers.ModelSerializer): + """ Serializer fot the Adherent objects (who belong to a + Mailing) """ + class Meta: model = Adherent fields = ('email',) diff --git a/users/tests.py b/users/tests.py index 21fa6d24..85a8e9f1 100644 --- a/users/tests.py +++ b/users/tests.py @@ -19,7 +19,10 @@ # You should have received a copy of the GNU General Public License along # with this program; if not, write to the Free Software Foundation, Inc., # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +"""users.tests +The tests for the Users module. +""" -from django.test import TestCase +# from django.test import TestCase # Create your tests here. diff --git a/users/views.py b/users/views.py index d82d5563..a7abf653 100644 --- a/users/views.py +++ b/users/views.py @@ -39,22 +39,37 @@ from django.urls import reverse from django.shortcuts import get_object_or_404, render, redirect from django.contrib import messages from django.contrib.auth.decorators import login_required, permission_required -from django.db.models import ProtectedError, Q -from django.db import IntegrityError +from django.db.models import ProtectedError from django.utils import timezone from django.db import transaction from django.http import HttpResponse from django.http import HttpResponseRedirect from django.views.decorators.csrf import csrf_exempt - from rest_framework.renderers import JSONRenderer - - -from reversion.models import Version from reversion import revisions as reversion -from users.serializers import MailingSerializer, MailingMemberSerializer -from users.models import ( + +from cotisations.models import Facture +from machines.models import Machine +from preferences.models import OptionalUser, GeneralOption, AssoOption +from re2o.views import form +from re2o.utils import ( + all_has_access, + SortTable, + re2o_paginator +) +from re2o.acl import ( + can_create, + can_edit, + can_delete_set, + can_delete, + can_view, + can_view_all, + can_change +) + +from .serializers import MailingSerializer, MailingMemberSerializer +from .models import ( User, Ban, Whitelist, @@ -66,7 +81,7 @@ from users.models import ( Club, ListShell, ) -from users.forms import ( +from .forms import ( BanForm, WhitelistForm, DelSchoolForm, @@ -86,25 +101,6 @@ from users.forms import ( ClubAdminandMembersForm, GroupForm ) -from cotisations.models import Facture -from machines.models import Machine -from preferences.models import OptionalUser, GeneralOption, AssoOption - -from re2o.views import form -from re2o.utils import ( - all_has_access, - SortTable, - re2o_paginator -) -from re2o.acl import ( - can_create, - can_edit, - can_delete_set, - can_delete, - can_view, - can_view_all, - can_change -) @can_create(Adherent) @@ -162,7 +158,7 @@ def new_club(request): @login_required @can_edit(Club) -def edit_club_admin_members(request, club_instance, clubid): +def edit_club_admin_members(request, club_instance, _clubid): """Vue d'edition de la liste des users administrateurs et membres d'un club""" club = ClubAdminandMembersForm( @@ -195,27 +191,27 @@ def edit_info(request, user, userid): si l'id est différent de request.user, vérifie la possession du droit cableur """ if user.is_class_adherent: - user = AdherentForm( + user_form = AdherentForm( request.POST or None, instance=user.adherent, user=request.user ) - elif user.is_class_club: - user = ClubForm( + else: + user_form = ClubForm( request.POST or None, instance=user.club, user=request.user ) - if user.is_valid(): - if user.changed_data: - user.save() + if user_form.is_valid(): + if user_form.changed_data: + user_form.save() messages.success(request, "L'user a bien été modifié") return redirect(reverse( 'users:profil', kwargs={'userid': str(userid)} )) return form( - {'userform': user, 'action_name': "Editer l'utilisateur"}, + {'userform': user_form, 'action_name': "Editer l'utilisateur"}, 'users/user.html', request ) @@ -226,21 +222,21 @@ def edit_info(request, user, userid): def state(request, user, userid): """ Changer l'etat actif/desactivé/archivé d'un user, need droit bureau """ - state = StateForm(request.POST or None, instance=user) - if state.is_valid(): - if state.changed_data: - if state.cleaned_data['state'] == User.STATE_ARCHIVE: + state_form = StateForm(request.POST or None, instance=user) + if state_form.is_valid(): + if state_form.changed_data: + if state_form.cleaned_data['state'] == User.STATE_ARCHIVE: user.archive() - elif state.cleaned_data['state'] == User.STATE_ACTIVE: + elif state_form.cleaned_data['state'] == User.STATE_ACTIVE: user.unarchive() - state.save() + state_form.save() messages.success(request, "Etat changé avec succès") return redirect(reverse( 'users:profil', kwargs={'userid': str(userid)} )) return form( - {'userform': state, 'action_name': "Editer l'état"}, + {'userform': state_form, 'action_name': "Editer l'état"}, 'users/user.html', request ) @@ -249,17 +245,18 @@ def state(request, user, userid): @login_required @can_edit(User, 'groups') def groups(request, user, userid): - group = GroupForm(request.POST or None, instance=user) - if group.is_valid(): - if group.changed_data: - group.save() + """ View to edit the groups of a user """ + group_form = GroupForm(request.POST or None, instance=user) + if group_form.is_valid(): + if group_form.changed_data: + group_form.save() messages.success(request, "Groupes changés avec succès") return redirect(reverse( 'users:profil', kwargs={'userid': str(userid)} )) return form( - {'userform': group, 'action_name': 'Editer les groupes'}, + {'userform': group_form, 'action_name': 'Editer les groupes'}, 'users/user.html', request ) @@ -278,7 +275,7 @@ def password(request, user, userid): messages.success(request, "Le mot de passe a changé") return redirect(reverse( 'users:profil', - kwargs={'userid': str(user.id)} + kwargs={'userid': str(userid)} )) return form( {'userform': u_form, 'action_name': 'Changer le mot de passe'}, @@ -289,7 +286,8 @@ def password(request, user, userid): @login_required @can_edit(User, 'groups') -def del_group(request, user, userid, listrightid): +def del_group(request, user, _userid, listrightid): + """ View used to delete a group """ user.groups.remove(ListRight.objects.get(id=listrightid)) user.save() messages.success(request, "Droit supprimé à %s" % user) @@ -319,7 +317,7 @@ def new_serviceuser(request): @login_required @can_edit(ServiceUser) -def edit_serviceuser(request, serviceuser, serviceuserid): +def edit_serviceuser(request, serviceuser, _serviceuserid): """ Edit a ServiceUser """ serviceuser = EditServiceUserForm( request.POST or None, @@ -342,7 +340,7 @@ def edit_serviceuser(request, serviceuser, serviceuserid): @login_required @can_delete(ServiceUser) -def del_serviceuser(request, serviceuser, serviceuserid): +def del_serviceuser(request, serviceuser, _serviceuserid): """Suppression d'un ou plusieurs serviceusers""" if request.method == "POST": serviceuser.delete() @@ -365,7 +363,7 @@ def add_ban(request, user, userid): ban_instance = Ban(user=user) ban = BanForm(request.POST or None, instance=ban_instance) if ban.is_valid(): - _ban_object = ban.save() + ban.save() messages.success(request, "Bannissement ajouté") return redirect(reverse( 'users:profil', @@ -385,7 +383,7 @@ def add_ban(request, user, userid): @login_required @can_edit(Ban) -def edit_ban(request, ban_instance, banid): +def edit_ban(request, ban_instance, _banid): """ Editer un bannissement, nécessite au moins le droit bofh (a fortiori bureau) Syntaxe : JJ/MM/AAAA , heure optionnelle, prend effet immédiatement""" @@ -436,7 +434,7 @@ def add_whitelist(request, user, userid): @login_required @can_edit(Whitelist) -def edit_whitelist(request, whitelist_instance, whitelistid): +def edit_whitelist(request, whitelist_instance, _whitelistid): """ Editer un accès gracieux, temporaire ou permanent. Need droit cableur Syntaxe : JJ/MM/AAAA , heure optionnelle, prend effet immédiatement, @@ -476,7 +474,7 @@ def add_school(request): @login_required @can_edit(School) -def edit_school(request, school_instance, schoolid): +def edit_school(request, school_instance, _schoolid): """ Editer un établissement d'enseignement à partir du schoolid dans la base de donnée, need cableur""" school = SchoolForm(request.POST or None, instance=school_instance) @@ -537,7 +535,7 @@ def add_shell(request): @login_required @can_edit(ListShell) -def edit_shell(request, shell_instance, listshellid): +def edit_shell(request, shell_instance, _listshellid): """ Editer un shell à partir du listshellid""" shell = ShellForm(request.POST or None, instance=shell_instance) if shell.is_valid(): @@ -554,7 +552,7 @@ def edit_shell(request, shell_instance, listshellid): @login_required @can_delete(ListShell) -def del_shell(request, shell, listshellid): +def del_shell(request, shell, _listshellid): """Destruction d'un shell""" if request.method == "POST": shell.delete() @@ -586,7 +584,7 @@ def add_listright(request): @login_required @can_edit(ListRight) -def edit_listright(request, listright_instance, listrightid): +def edit_listright(request, listright_instance, _listrightid): """ Editer un groupe/droit, necessite droit bureau, à partir du listright id """ listright = ListRightForm( @@ -800,7 +798,7 @@ def mon_profil(request): @login_required @can_view(User) -def profil(request, users, userid): +def profil(request, users, _userid): """ Affiche un profil, self or cableur, prend un userid en argument """ machines = Machine.objects.filter(user=users).select_related('user')\ .prefetch_related('interface_set__domain__extension')\ @@ -924,7 +922,7 @@ class JSONResponse(HttpResponse): @csrf_exempt @login_required @permission_required('machines.serveur') -def ml_std_list(request): +def ml_std_list(_request): """ API view sending all the available standard mailings""" return JSONResponse([ {'name': 'adherents'} @@ -950,7 +948,7 @@ def ml_std_members(request, ml_name): @csrf_exempt @login_required @permission_required('machines.serveur') -def ml_club_list(request): +def ml_club_list(_request): """ API view sending all the available club mailings""" clubs = Club.objects.filter(mailing=True).values('pseudo') seria = MailingSerializer(clubs, many=True)