Sort ACL app views. Don't error out on duplicate ACL entry creation (same document + role), but instead redirect to existing ACL entry.

mayan/apps/acls/views.py

@@ -3,10 +3,11 @@ from __future__ import absolute_import, unicode_literals
 import itertools
 import logging
 
+from django.contrib import messages
 from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import PermissionDenied
 from django.core.urlresolvers import reverse
-from django.http import Http404
+from django.http import Http404, HttpResponseRedirect
 from django.shortcuts import get_object_or_404
 from django.utils.translation import ugettext_lazy as _
 
@@ -24,7 +25,10 @@ from .permissions import permission_acl_edit, permission_acl_view
 logger = logging.getLogger(__name__)
 
 
-class ACLListView(SingleObjectListView):
+class ACLCreateView(SingleObjectCreateView):
+    fields = ('role',)
+    model = AccessControlList
+
     def dispatch(self, request, *args, **kwargs):
         self.content_type = get_object_or_404(
             ContentType, app_label=self.kwargs['app_label'],
@@ -38,47 +42,6 @@ class ACLListView(SingleObjectListView):
         except self.content_type.model_class().DoesNotExist:
             raise Http404
 
-        try:
-            Permission.check_permissions(
-                request.user, permissions=(permission_acl_view,)
-            )
-        except PermissionDenied:
-            AccessControlList.objects.check_access(
-                permission_acl_view, request.user, self.content_object
-            )
-
-        return super(ACLListView, self).dispatch(request, *args, **kwargs)
-
-    def get_queryset(self):
-        return AccessControlList.objects.filter(
-            content_type=self.content_type, object_id=self.content_object.pk
-        )
-
-    def get_extra_context(self):
-        return {
-            'hide_object': True,
-            'object': self.content_object,
-            'title': _('Access control lists for: %s' % self.content_object),
-        }
-
-
-class ACLCreateView(SingleObjectCreateView):
-    fields = ('role',)
-    model = AccessControlList
-
-    def dispatch(self, request, *args, **kwargs):
-        content_type = get_object_or_404(
-            ContentType, app_label=self.kwargs['app_label'],
-            model=self.kwargs['model']
-        )
-
-        try:
-            self.content_object = content_type.get_object_for_this_type(
-                pk=self.kwargs['object_id']
-            )
-        except content_type.model_class().DoesNotExist:
-            raise Http404
-
         try:
             Permission.check_permissions(
                 request.user, permissions=(permission_acl_edit,)
@@ -90,15 +53,24 @@ class ACLCreateView(SingleObjectCreateView):
 
         return super(ACLCreateView, self).dispatch(request, *args, **kwargs)
 
+    def get_instance_extra_data(self):
+        return {
+            'content_object': self.content_object
+        }
+
     def form_valid(self, form):
-        self.instance = form.save(commit=False)
-        self.instance.content_object = self.content_object
-        self.instance.save()
-
-        return super(ACLCreateView, self).form_valid(form)
-
-    def get_success_url(self):
-        return reverse('acls:acl_permissions', args=(self.instance.pk,))
+        try:
+            acl = AccessControlList.objects.get(
+                content_type=self.content_type,
+                object_id=self.content_object.pk,
+                role=form.cleaned_data['role']
+            )
+        except AccessControlList.DoesNotExist:
+            return super(ACLCreateView, self).form_valid(form)
+        else:
+            return HttpResponseRedirect(
+                reverse('acls:acl_permissions', args=(acl.pk,))
+            )
 
     def get_extra_context(self):
         return {
@@ -108,6 +80,12 @@ class ACLCreateView(SingleObjectCreateView):
             ) % self.content_object
         }
 
+    def get_success_url(self):
+        if self.object.pk:
+            return reverse('acls:acl_permissions', args=(self.object.pk,))
+        else:
+            return super(ACLCreateView, self).get_success_url()
+
 
 class ACLDeleteView(SingleObjectDeleteView):
     model = AccessControlList
@@ -142,6 +120,44 @@ class ACLDeleteView(SingleObjectDeleteView):
         )
 
 
+class ACLListView(SingleObjectListView):
+    def dispatch(self, request, *args, **kwargs):
+        self.content_type = get_object_or_404(
+            ContentType, app_label=self.kwargs['app_label'],
+            model=self.kwargs['model']
+        )
+
+        try:
+            self.content_object = self.content_type.get_object_for_this_type(
+                pk=self.kwargs['object_id']
+            )
+        except self.content_type.model_class().DoesNotExist:
+            raise Http404
+
+        try:
+            Permission.check_permissions(
+                request.user, permissions=(permission_acl_view,)
+            )
+        except PermissionDenied:
+            AccessControlList.objects.check_access(
+                permission_acl_view, request.user, self.content_object
+            )
+
+        return super(ACLListView, self).dispatch(request, *args, **kwargs)
+
+    def get_extra_context(self):
+        return {
+            'hide_object': True,
+            'object': self.content_object,
+            'title': _('Access control lists for: %s' % self.content_object),
+        }
+
+    def get_queryset(self):
+        return AccessControlList.objects.filter(
+            content_type=self.content_type, object_id=self.content_object.pk
+        )
+
+
 class ACLPermissionsView(AssignRemoveView):
     grouped = True
     left_list_title = _('Available permissions')
@@ -181,17 +197,6 @@ class ACLPermissionsView(AssignRemoveView):
             ACLPermissionsView, self
         ).dispatch(request, *args, **kwargs)
 
-    def get_right_list_help_text(self):
-        if self.get_object().get_inherited_permissions():
-            return _(
-                'Disabled permissions are inherited from a parent object.'
-            )
-
-        return None
-
-    def get_object(self):
-        return get_object_or_404(AccessControlList, pk=self.kwargs['pk'])
-
     def get_available_list(self):
         return ModelPermission.get_for_instance(
             instance=self.get_object().content_object
@@ -229,12 +234,23 @@ class ACLPermissionsView(AssignRemoveView):
         merged_pks = self.get_object().permissions.values_list('pk', flat=True) | self.get_object().get_inherited_permissions().values_list('pk', flat=True)
         return StoredPermission.objects.filter(pk__in=merged_pks)
 
+    def get_object(self):
+        return get_object_or_404(AccessControlList, pk=self.kwargs['pk'])
+
+    def get_right_list_help_text(self):
+        if self.get_object().get_inherited_permissions():
+            return _(
+                'Disabled permissions are inherited from a parent object.'
+            )
+
+        return None
+
     def left_list(self):
         return ACLPermissionsView.generate_choices(self.get_available_list())
 
-    def right_list(self):
-        return ACLPermissionsView.generate_choices(self.get_granted_list())
-
     def remove(self, item):
         permission = get_object_or_404(StoredPermission, pk=item)
         self.get_object().permissions.remove(permission)
+
+    def right_list(self):
+        return ACLPermissionsView.generate_choices(self.get_granted_list())