From eaf960902478b53528ff8134fd63d96d37933ef7 Mon Sep 17 00:00:00 2001
From: Hugo Levy-Falk <hugo.levy--falk@supelec.fr>
Date: Fri, 6 Sep 2019 01:08:53 +0200
Subject: [PATCH] helpful acl messages structure

---
 re2o/acl.py   | 40 +++++++++++++++++++++++++++++++---------
 re2o/utils.py | 11 +++++++++++
 2 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/re2o/acl.py b/re2o/acl.py
index 59b09716..5d3601b4 100644
--- a/re2o/acl.py
+++ b/re2o/acl.py
@@ -36,6 +36,22 @@ from django.shortcuts import redirect
 from django.urls import reverse
 from django.utils.translation import ugettext as _
 
+from re2o.utils import get_group_having_permission
+
+
+def group_list(permissions):
+    """Create a string listing every groups having one of the given
+    `permissions`."""
+    if permissions:
+        return ", ".join([
+            g.name for g in get_group_having_permission(*permissions)
+        ]) or "No group have the %s permission(s) !" % " or ".join([
+            ",".join(permissions[:-1]),
+            permissions[-1]] if len(permissions) > 2 else permissions
+        )
+    else:
+        return ""
+
 
 def acl_base_decorator(method_name, *targets, on_instance=True):
     """Base decorator for acl. It checks if the `request.user` has the
@@ -53,9 +69,10 @@ def acl_base_decorator(method_name, *targets, on_instance=True):
             then no error will be triggered, the decorator will act as if
             permission was granted. This is to allow you to run ACL tests on
             fields only. If the method exists, it has to return a 2-tuple
-            `(can, reason)` with `can` being a boolean stating whether the
-            access is granted and `reason` a message to be displayed if `can`
-            equals `False` (can be `None`)
+            `(can, reason, permissions)` with `can` being a boolean stating
+            whether the access is granted, `reason` a message to be
+            displayed if `can` equals `False` (can be `None`) and `permissions`
+            a list of permissions needed for access (can be `None`).
         *targets: The targets. Targets are specified like a sequence of models
             and fields names. As an example
             ```
@@ -139,7 +156,7 @@ ModelC)
                         target = target.get_instance(*args, **kwargs)
                         instances.append(target)
                     except target.DoesNotExist:
-                        yield False, _("Nonexistent entry.")
+                        yield False, _("Nonexistent entry."), []
                         return
                 if hasattr(target, method_name):
                     can_fct = getattr(target, method_name)
@@ -148,11 +165,16 @@ ModelC)
                     can_change_fct = getattr(target, 'can_change_' + field)
                     yield can_change_fct(request.user, *args, **kwargs)
 
-            error_messages = [
-                x[1] for x in chain.from_iterable(
-                    process_target(x[0], x[1]) for x in group_targets()
-                ) if not x[0]
-            ]
+            error_messages = []
+            for target, fields in group_targets():
+                for can, msg, permissions in process_target(target, fields):
+                    if not can:
+                        error_messages.append(
+                            msg + _(
+                                " You need to be a member of one of those"
+                                " groups : %s"
+                            ) % group_list(permissions)
+                        )
             if error_messages:
                 for msg in error_messages:
                     messages.error(
diff --git a/re2o/utils.py b/re2o/utils.py
index 2470cf50..a5cb2238 100644
--- a/re2o/utils.py
+++ b/re2o/utils.py
@@ -38,12 +38,23 @@ from __future__ import unicode_literals
 
 from django.utils import timezone
 from django.db.models import Q
+from django.contrib.auth.models import Permission
 
 from cotisations.models import Cotisation, Facture, Vente
 from machines.models import Interface, Machine
 from users.models import Adherent, User, Ban, Whitelist
 from preferences.models import AssoOption
 
+def get_group_having_permission(*permission_name):
+    """Returns every group having the permission `permission_name`
+    """
+    groups = set()
+    for name in permission_name:
+        app_label, codename = name.split('.')
+        permission = Permission.objects.get(content_type__app_label=app_label, codename=codename)
+        groups = groups.union(permission.group_set.all())
+    return groups
+
 def all_adherent(search_time=None, including_asso=True):
     """ Fonction renvoyant tous les users adherents. Optimisee pour n'est
     qu'une seule requete sql