Merge branch 'Aufinal/kfet-auth' into 'master'

Groupes et perms K-Fêt

See merge request klub-dev-ens/gestioCOF!438

kfet/auth/fields.py

@@ -1,17 +1,24 @@
 from django import forms
-from django.contrib.auth.models import Permission
+from django.contrib.auth.models import Group
-from django.contrib.contenttypes.models import ContentType
+
-from django.forms import widgets
+from .models import KFetPermission
 
 
 class KFetPermissionsField(forms.ModelMultipleChoiceField):
     def __init__(self, *args, **kwargs):
-        queryset = Permission.objects.filter(
+        kwargs.setdefault("queryset", KFetPermission.objects.all())
-            content_type__in=ContentType.objects.filter(app_label="kfet")
+        kwargs.setdefault("widget", forms.CheckboxSelectMultiple)
-        )
+        super().__init__(*args, **kwargs)
-        super().__init__(
+
-            queryset=queryset, widget=widgets.CheckboxSelectMultiple, *args, **kwargs
+    def label_from_instance(self, obj):
-        )
+        return obj.name
+
+
+class KFetGroupsField(forms.ModelMultipleChoiceField):
+    def __init__(self, *args, **kwargs):
+        kwargs.setdefault("queryset", Group.objects.filter(kfetgroup__isnull=False))
+        kwargs.setdefault("widget", forms.SelectMultiple)
+        super().__init__(*args, **kwargs)
 
     def label_from_instance(self, obj):
         return obj.name

kfet/auth/forms.py

@@ -1,48 +1,29 @@
-from django import forms
+from django.contrib.auth.models import User
-from django.contrib.auth.models import Group, User
+from django.utils.translation import ugettext_lazy as _
 
-from .fields import KFetPermissionsField
+from shared.forms import ProtectedModelForm
+
+from .fields import KFetGroupsField, KFetPermissionsField
+from .models import KFetGroup
 
 
-class GroupForm(forms.ModelForm):
+class GroupForm(ProtectedModelForm):
     permissions = KFetPermissionsField()
 
-    def clean_name(self):
+    protected_fields = ["permissions"]
-        name = self.cleaned_data["name"]
-        return "K-Fêt %s" % name
-
-    def clean_permissions(self):
-        kfet_perms = self.cleaned_data["permissions"]
-        # TODO: With Django >=1.11, the QuerySet method 'difference' can be
-        # used.
-        # other_groups = self.instance.permissions.difference(
-        #     self.fields['permissions'].queryset
-        # )
-        if self.instance.pk is None:
-            return kfet_perms
-        other_perms = self.instance.permissions.exclude(
-            pk__in=[p.pk for p in self.fields["permissions"].queryset]
-        )
-        return list(kfet_perms) + list(other_perms)
 
     class Meta:
-        model = Group
+        model = KFetGroup
         fields = ["name", "permissions"]
 
 
-class UserGroupForm(forms.ModelForm):
+class UserGroupForm(ProtectedModelForm):
-    groups = forms.ModelMultipleChoiceField(
+    groups = KFetGroupsField(
-        Group.objects.filter(name__icontains="K-Fêt"),
+        label=_("Statut équipe"),
-        label="Statut équipe",
         required=False,
     )
 
-    def clean_groups(self):
+    protected_fields = ["groups"]
-        kfet_groups = self.cleaned_data.get("groups")
-        if self.instance.pk is None:
-            return kfet_groups
-        other_groups = self.instance.groups.exclude(name__icontains="K-Fêt")
-        return list(kfet_groups) + list(other_groups)
 
     class Meta:
         model = User

kfet/auth/migrations/0002_kfetgroup_kfetpermission.py

@@ -0,0 +1,55 @@
+# Generated by Django 2.2.8 on 2020-01-08 21:03
+
+import django.contrib.auth.models
+import django.db.models.deletion
+import django.db.models.manager
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("auth", "0011_update_proxy_permissions"),
+        ("kfetauth", "0001_initial"),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name="KFetGroup",
+            fields=[
+                (
+                    "group_ptr",
+                    models.OneToOneField(
+                        auto_created=True,
+                        on_delete=django.db.models.deletion.CASCADE,
+                        parent_link=True,
+                        primary_key=True,
+                        serialize=False,
+                        to="auth.Group",
+                    ),
+                ),
+            ],
+            options={
+                "verbose_name": "Groupe K-Fêt",
+                "verbose_name_plural": "Groupes K-Fêt",
+            },
+            bases=("auth.group",),
+            managers=[("objects", django.contrib.auth.models.GroupManager())],
+        ),
+        migrations.CreateModel(
+            name="KFetPermission",
+            fields=[],
+            options={
+                "verbose_name": "Permission K-Fêt",
+                "verbose_name_plural": "Permissions K-Fêt",
+                "proxy": True,
+                "indexes": [],
+                "constraints": [],
+            },
+            bases=("auth.permission",),
+            managers=[
+                ("kfet", django.db.models.manager.Manager()),
+                ("objects", django.contrib.auth.models.PermissionManager()),
+            ],
+        ),
+    ]

kfet/auth/migrations/0003_existing_groups.py

@@ -0,0 +1,21 @@
+# Generated by Django 2.2.8 on 2020-01-08 21:04
+
+from django.db import migrations
+
+
+def existing_groups(apps, schema_editor):
+    Group = apps.get_model("auth", "Group")
+    KFetGroup = apps.get_model("kfetauth", "KFetGroup")
+
+    for group in Group.objects.filter(name__icontains="K-Fêt"):
+        kf_group = KFetGroup(group_ptr=group, pk=group.pk, name=group.name)
+        kf_group.save()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("kfetauth", "0002_kfetgroup_kfetpermission"),
+    ]
+
+    operations = [migrations.RunPython(existing_groups)]

kfet/auth/models.py

@@ -1,5 +1,9 @@
+from django.contrib.auth.models import Group, Permission
 from django.db import models
 from django.utils.crypto import get_random_string
+from django.utils.translation import ugettext_lazy as _
+
+KFET_APP_LABELS = ["kfet", "kfetauth"]
 
 
 class GenericTeamTokenManager(models.Manager):
@@ -14,3 +18,28 @@ class GenericTeamToken(models.Model):
     token = models.CharField(max_length=50, unique=True)
 
     objects = GenericTeamTokenManager()
+
+
+class KFetPermissionManager(models.Manager):
+    def get_queryset(self):
+        return (
+            super().get_queryset().filter(content_type__app_label__in=KFET_APP_LABELS)
+        )
+
+
+class KFetPermission(Permission):
+    objects = KFetPermissionManager()
+
+    class Meta:
+        proxy = True
+        verbose_name = _("Permission K-Fêt")
+        verbose_name_plural = _("Permissions K-Fêt")
+
+
+class KFetGroup(Group):
+    # On fait un héritage complet pour
+    # mieux distinguer les groupes K-Fêt via l'ORM (i.e. faire `KFetGroup.objects.all`)
+
+    class Meta:
+        verbose_name = _("Groupe K-Fêt")
+        verbose_name_plural = _("Groupes K-Fêt")

kfet/auth/tests.py

@@ -10,7 +10,7 @@ from kfet.models import Account
 
 from . import KFET_GENERIC_TRIGRAMME, KFET_GENERIC_USERNAME
 from .backends import AccountBackend, GenericBackend
-from .models import GenericTeamToken
+from .models import GenericTeamToken, KFetGroup
 from .utils import get_kfet_generic_user
 from .views import GenericLoginView
 
@@ -27,11 +27,8 @@ class UserGroupFormTests(TestCase):
         self.user = User.objects.create(username="foo", password="foo")
 
         # create some K-Fêt groups
-        prefix_name = "K-Fêt "
         names = ["Group 1", "Group 2", "Group 3"]
-        self.kfet_groups = [
+        self.kfet_groups = [KFetGroup.objects.create(name=name) for name in names]
-            Group.objects.create(name=prefix_name + name) for name in names
-        ]
 
         # create a non-K-Fêt group
         self.other_group = Group.objects.create(name="Other group")
@@ -41,7 +38,9 @@ class UserGroupFormTests(TestCase):
         form = UserGroupForm(instance=self.user)
         groups_field = form.fields["groups"]
         self.assertQuerysetEqual(
-            groups_field.queryset, [repr(g) for g in self.kfet_groups], ordered=False
+            groups_field.queryset,
+            [repr(g.group_ptr) for g in self.kfet_groups],
+            ordered=False,
         )
 
     def test_keep_others(self):
@@ -59,7 +58,8 @@ class UserGroupFormTests(TestCase):
         form.save()
         self.assertQuerysetEqual(
             user.groups.all(),
-            [repr(g) for g in [self.other_group] + self.kfet_groups],
+            [self.other_group.pk] + [group.pk for group in self.kfet_groups],
+            transform=lambda group: group.pk,
             ordered=False,
         )
 

kfet/auth/views.py

@@ -1,7 +1,7 @@
 from django.contrib import messages
 from django.contrib.auth import authenticate, login
 from django.contrib.auth.decorators import permission_required
-from django.contrib.auth.models import Group, User
+from django.contrib.auth.models import User
 from django.contrib.auth.views import redirect_to_login
 from django.contrib.messages.views import SuccessMessageMixin
 from django.db.models import Prefetch
@@ -15,7 +15,7 @@ from django.views.generic import View
 from django.views.generic.edit import CreateView, UpdateView
 
 from .forms import GroupForm
-from .models import GenericTeamToken
+from .models import GenericTeamToken, KFetGroup
 
 
 class GenericLoginView(View):
@@ -113,23 +113,20 @@ def account_group(request):
     user_pre = Prefetch(
         "user_set", queryset=User.objects.select_related("profile__account_kfet")
     )
-    groups = Group.objects.filter(name__icontains="K-Fêt").prefetch_related(
+    groups = KFetGroup.objects.prefetch_related("permissions", user_pre)
-        "permissions", user_pre
-    )
     return render(request, "kfet/account_group.html", {"groups": groups})
 
 
-class AccountGroupCreate(SuccessMessageMixin, CreateView):
+class AccountGroupFormMixin(SuccessMessageMixin):
-    model = Group
+    model = KFetGroup
     template_name = "kfet/account_group_form.html"
     form_class = GroupForm
+    success_url = reverse_lazy("kfet.account.group")
+
+
+class AccountGroupCreate(AccountGroupFormMixin, CreateView):
     success_message = "Nouveau groupe : %(name)s"
-    success_url = reverse_lazy("kfet.account.group")
 
 
-class AccountGroupUpdate(SuccessMessageMixin, UpdateView):
+class AccountGroupUpdate(AccountGroupFormMixin, UpdateView):
-    queryset = Group.objects.filter(name__icontains="K-Fêt")
-    template_name = "kfet/account_group_form.html"
-    form_class = GroupForm
     success_message = "Groupe modifié : %(name)s"
-    success_url = reverse_lazy("kfet.account.group")

kfet/management/commands/loadkfetdevdata.py

@@ -6,12 +6,13 @@ import os
 import random
 from datetime import timedelta
 
-from django.contrib.auth.models import Group, Permission, User
+from django.contrib.auth.models import User
 from django.core.management import call_command
 from django.utils import timezone
 
 from gestioncof.management.base import MyBaseCommand
 from gestioncof.models import CofProfile
+from kfet.auth.models import KFetGroup, KFetPermission
 from kfet.models import (
     Account,
     Article,
@@ -33,23 +34,17 @@ class Command(MyBaseCommand):
         # Groupes
         # ---
 
-        Group.objects.filter(name__icontains="K-Fêt").delete()
+        group_chef, _ = KFetGroup.objects.get_or_create(name="K-Fêt César")
+        group_boy, _ = KFetGroup.objects.get_or_create(name="K-Fêt Légionnaire")
 
-        group_chef = Group(name="K-Fêt César")
+        # Give relevant permissions to both groups
-        group_boy = Group(name="K-Fêt Légionnaire")
+        chef_perms = KFetPermission.objects.all()
+        group_chef.permissions.add(*chef_perms)
 
-        group_chef.save()
+        boy_perms = KFetPermission.objects.filter(
-        group_boy.save()
+            codename__in=["is_team", "perform_deposit", "add_account", "add_transfer"]
-
-        permissions_chef = Permission.objects.filter(
-            content_type__app_label="kfet",
         )
-        permissions_boy = Permission.objects.filter(
+        group_boy.permissions.add(*boy_perms)
-            content_type__app_label="kfet", codename__in=["is_team", "perform_deposit"]
-        )
-
-        group_chef.permissions.add(*permissions_chef)
-        group_boy.permissions.add(*permissions_boy)
 
         # ---
         # Comptes

kfet/templates/kfet/account_group_form.html

@@ -7,35 +7,6 @@
 
 {% block main %}
 
-<form action="" method="post" class="form-horizontal">
+{% include "kfet/form_full_snippet.html" with authz=perms.kfet.manage_perms submit_text="Enregistrer" %}
-  {% csrf_token %}
-  <div class="form-group">
-    <label for="{{ form.name.id_for_label }}" class="col-sm-2 control-label">{{ form.name.label }}</label>
-    <div class="col-sm-10">
-      <div class="input-group">
-        <span class="input-group-addon">K-Fêt</span>
-        {{ form.name|add_class:"form-control" }}
-      </div>
-      {% if form.name.errors %}
-        <span class="help-block">{{ form.name.errors }}</span>
-      {% endif %}
-      {% if form.name.help_text %}
-        <span class="help-block">{{ form.name.help_text }}</span>
-      {% endif %}
-    </div>
-  </div>
-  {% include "kfet/form_field_snippet.html" with field=form.permissions %}
-  {% include "kfet/form_submit_snippet.html" with value="Enregistrer" %}
-</form>
-
-<script type="text/javascript">
-$(document).ready(function() {
-    let $name_input = $("#id_name");
-    let raw_name = $name_input.val();
-    let prefix = "K-Fêt ";
-    if (raw_name.startsWith(prefix))
-        $name_input.val(raw_name.substring(prefix.length));
-});
-</script>
 
 {% endblock %}

kfet/templates/kfet/form_field_snippet.html

@@ -5,7 +5,7 @@
   <div class="col-sm-10">
     {% if field|widget_type == "checkboxselectmultiple" %}
       <ul class="list-unstyled checkbox-select-multiple">
-        {% for choice in form.permissions %}
+        {% for choice in field %}
         <li class="col-sm-6 col-lg-4">
           <label for="{{ choice.id_for_label }}">
             {{ choice.tag }} {{ choice.choice_label }}

kfet/tests/test_views.py

@@ -3,13 +3,14 @@ from datetime import datetime, timedelta
 from decimal import Decimal
 from unittest import mock
 
-from django.contrib.auth.models import Group, User
+from django.contrib.auth.models import User
 from django.test import Client, TestCase
 from django.urls import reverse
 from django.utils import timezone
 
 from .. import KFET_DELETED_TRIGRAMME
 from ..auth import KFET_GENERIC_TRIGRAMME
+from ..auth.models import KFetGroup
 from ..config import kfet_config
 from ..models import (
     Account,
@@ -464,15 +465,18 @@ class AccountGroupListViewTests(ViewTestCaseMixin, TestCase):
 
     def setUp(self):
         super().setUp()
-        self.group1 = Group.objects.create(name="K-Fêt - Group1")
+        self.group1 = KFetGroup.objects.create(name="Group1")
-        self.group2 = Group.objects.create(name="K-Fêt - Group2")
+        self.group2 = KFetGroup.objects.create(name="Group2")
 
     def test_ok(self):
         r = self.client.get(self.url)
         self.assertEqual(r.status_code, 200)
 
         self.assertQuerysetEqual(
-            r.context["groups"], map(repr, [self.group1, self.group2]), ordered=False
+            r.context["groups"],
+            [self.group1.pk, self.group2.pk],
+            transform=lambda group: group.pk,
+            ordered=False,
         )
 
 
@@ -510,7 +514,7 @@ class AccountGroupCreateViewTests(ViewTestCaseMixin, TestCase):
         r = self.client.post(self.url, self.post_data)
         self.assertRedirects(r, reverse("kfet.account.group"))
 
-        group = Group.objects.get(name="K-Fêt The Group")
+        group = KFetGroup.objects.get(name="The Group")
 
         self.assertQuerysetEqual(
             group.permissions.all(),
@@ -551,7 +555,7 @@ class AccountGroupUpdateViewTests(ViewTestCaseMixin, TestCase):
     def setUp(self):
         super().setUp()
         self.perms = get_perms("kfet.is_team", "kfet.manage_perms")
-        self.group = Group.objects.create(name="K-Fêt - Group")
+        self.group = KFetGroup.objects.create(name="Group")
         self.group.permissions.set(self.perms.values())
 
     def test_get_ok(self):
@@ -564,7 +568,7 @@ class AccountGroupUpdateViewTests(ViewTestCaseMixin, TestCase):
 
         self.group.refresh_from_db()
 
-        self.assertEqual(self.group.name, "K-Fêt The Group")
+        self.assertEqual(self.group.name, "The Group")
         self.assertQuerysetEqual(
             self.group.permissions.all(),
             map(repr, [self.perms["kfet.is_team"], self.perms["kfet.manage_perms"]]),

shared/forms.py

@@ -0,0 +1,50 @@
+from django.forms.models import ModelForm
+
+
+class ProtectedModelForm(ModelForm):
+    """
+    Extension de `ModelForm`
+
+    Quand on save un champ `ManyToMany` dans un `ModelForm`, la méthode appelée
+    est <field>.set(), qui écrase l'intégralité du contenu.
+    Le problème survient quand le `field` a un queryset restreint, et qu'on ne
+    veut pas toucher aux choix qui ne sont pas dans ce queryset...
+    C'est le but de ce mixin.
+
+    Attributs :
+        - `protected_fields` : champs qu'on souhaite protéger.
+    """
+
+    protected_fields = []
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
+        for field_name in self.protected_fields:
+            if field_name not in self.fields:
+                raise ValueError("Le champ %s n'existe pas !" % field_name)
+
+    def _get_protected_elts(self, field_name):
+        """
+        Renvoie tous les éléments de `instance.<field_name>` qui ne sont pas
+        dans `self.<field_name>.queryset` (et sont donc à conserver).
+
+        NB : on "désordonne" tous les querysets via `.order_by()` car Django
+        ne peut pas effectuer une union de QS ordonnés.
+        """
+        if self.instance.pk:
+            previous = getattr(self.instance, field_name).order_by()
+            selectable = self.fields[field_name].queryset.order_by()
+            return previous.difference(selectable)
+        else:
+            # Nouvelle instance, rien à protéger.
+            return self.fields[field_name].queryset.none()
+
+    def clean(self):
+        cleaned_data = super().clean()
+        for field_name in self.protected_fields:
+            selected_elts = cleaned_data[field_name].order_by()
+            protected_elts = self._get_protected_elts(field_name)
+            cleaned_data[field_name] = selected_elts.union(protected_elts)
+
+        return cleaned_data