♻ permissions refactoring: refactor has_permission methods

5 files changed, 126 insertions, 79 deletions

diff --git a/archaeological_warehouse/ishtar_menu.py b/archaeological_warehouse/ishtar_menu.py
index 851ec3643..c0b8d658b 100644
--- a/archaeological_warehouse/ishtar_menu.py
+++ b/archaeological_warehouse/ishtar_menu.py
@@ -23,7 +23,7 @@ from ishtar_common.menu_base import SectionItem, MenuItem
 
 from . import models
 
-# be careful: each access_controls must be relevant with check_rights in urls
+# be careful: each access_controls must be relevant with check_permissions in urls
 
 
 MENU_SECTIONS = [
@@ -39,14 +39,17 @@ MENU_SECTIONS = [
                     "warehouse_search",
                     _("Search"),
                     model=models.Warehouse,
-                    access_controls=["view_warehouse", "view_own_warehouse"],
+                    access_controls=[
+                        "archaeological_warehouse.view_warehouse",
+                        "archaeological_warehouse.view_own_warehouse"
+                    ],
                 ),
                 MenuItem(
                     "warehouse_creation",
                     _("Creation"),
                     model=models.Warehouse,
                     access_controls=[
-                        "add_warehouse",
+                        "archaeological_warehouse.add_warehouse",
                     ],
                 ),
                 MenuItem(
@@ -54,7 +57,7 @@ MENU_SECTIONS = [
                     _("Modification"),
                     model=models.Warehouse,
                     access_controls=[
-                        "change_warehouse",
+                        "archaeological_warehouse.change_warehouse",
                     ],
                 ),
                 MenuItem(
@@ -62,7 +65,7 @@ MENU_SECTIONS = [
                     _("Deletion"),
                     model=models.Warehouse,
                     access_controls=[
-                        "change_warehouse",
+                        "archaeological_warehouse.change_warehouse",
                     ],
                 ),
             ],
@@ -80,42 +83,48 @@ MENU_SECTIONS = [
                     "container_search",
                     _("Search"),
                     model=models.Warehouse,
-                    access_controls=["view_container", "view_own_container"],
+                    access_controls=[
+                        "archaeological_warehouse.view_container",
+                        "archaeological_warehouse.view_own_container"
+                    ],
                 ),
                 MenuItem(
                     "container_creation",
                     _("Creation"),
                     model=models.Warehouse,
-                    access_controls=["add_container", "add_own_container"],
+                    access_controls=[
+                        "archaeological_warehouse.add_container",
+                        "archaeological_warehouse.add_own_container"
+                    ],
                 ),
                 MenuItem(
                     "container_modification",
                     _("Modification"),
                     model=models.Warehouse,
                     access_controls=[
-                        "change_container",
-                        "change_own_container",
+                        "archaeological_warehouse.change_container",
+                        "archaeological_warehouse.change_own_container",
                     ],
                 ),
                 MenuItem(
                     "container-merge",
                     _("Automatic merge"),
                     model=models.Container,
-                    access_controls=["administrator"],
+                    access_controls=["ishtaradmin"],
                 ),
                 MenuItem(
                     "container-manual-merge",
                     _("Manual merge"),
                     model=models.Container,
-                    access_controls=["administrator"],
+                    access_controls=["ishtaradmin"],
                 ),
                 MenuItem(
                     "container_deletion",
                     _("Deletion"),
                     model=models.Warehouse,
                     access_controls=[
-                        "change_container",
-                        "change_own_container",
+                        "archaeological_warehouse.change_container",
+                        "archaeological_warehouse.change_own_container",
                     ],
                 ),
             ],
diff --git a/archaeological_warehouse/models.py b/archaeological_warehouse/models.py
index 3cb04816d..9aa5549db 100644
--- a/archaeological_warehouse/models.py
+++ b/archaeological_warehouse/models.py
@@ -32,7 +32,8 @@ from django.urls import reverse
 from ishtar_common.utils import ugettext_lazy as _, pgettext_lazy
 
 from django.apps import apps
-from ishtar_common.data_importer import post_importer_action, pre_importer_action, ImporterError
+from ishtar_common.data_importer import post_importer_action, pre_importer_action, \
+                                        ImporterError
 from ishtar_common.model_managers import UUIDModelManager
 from ishtar_common.models import ValueGetter, get_current_profile, HistoryModel, Imported
 from ishtar_common.models_common import (
@@ -377,7 +378,10 @@ class Warehouse(
         icon_class="fa fa-pencil",
         text=_("Bulk update"),
         target="many",
-        rights=["change_warehouse", "change_own_warhouse"],
+        rights=[
+            "archaeological_warehouse.change_warehouse",
+            "archaeological_warehouse.change_own_warhouse"
+        ],
     )
 
     QA_LOCK = QuickAction(
@@ -385,7 +389,10 @@ class Warehouse(
         icon_class="fa fa-lock",
         text=_("Lock/Unlock"),
         target="many",
-        rights=["change_warehouse", "change_own_warehouse"],
+        rights=[
+            "archaeological_warehouse.change_warehouse",
+            "archaeological_warehouse.change_own_warehouse"
+        ],
     )
     QUICK_ACTIONS = [
         QA_LOCK,
@@ -1105,21 +1112,30 @@ class Container(
         icon_class="fa fa-pencil",
         text=_("Bulk update"),
         target="many",
-        rights=["change_container", "change_own_container"],
+        rights=[
+            "archaeological_warehouse.change_container",
+            "archaeological_warehouse.change_own_container"
+        ],
     )
     QA_MOVE = QuickAction(
         url="container-qa-move",
         icon_class="fa fa-arrow-right",
         text=pgettext_lazy("action", "Move"),
         target="many",
-        rights=["change_container", "change_own_container"],
+        rights=[
+            "archaeological_warehouse.change_container",
+            "archaeological_warehouse.change_own_container"
+        ],
     )
     QA_LOCK = QuickAction(
         url="container-qa-lock",
         icon_class="fa fa-lock",
         text=_("Lock/Unlock"),
         target="many",
-        rights=["change_container", "change_own_container"],
+        rights=[
+            "archaeological_warehouse.change_container",
+            "archaeological_warehouse.change_own_container"
+        ],
     )
     QUICK_ACTIONS = [QA_EDIT, QA_MOVE, QA_LOCK]
 
@@ -1939,7 +1955,7 @@ class Container(
         """
         # url, base_text, icon, extra_text, extra css class, is a quick action
         actions = super(Container, self).get_extra_actions(request)
-        can_edit_find = self.can_do(request, "change_find")
+        can_edit_find = self.can_do(request, "archaeological_finds.change_find")
         if can_edit_find:
             actions += [
                 (
diff --git a/archaeological_warehouse/tests.py b/archaeological_warehouse/tests.py
index c218b7d0e..96df83024 100644
--- a/archaeological_warehouse/tests.py
+++ b/archaeological_warehouse/tests.py
@@ -20,7 +20,7 @@
 import datetime
 import json
 
-from django.contrib.auth.models import Permission
+from django.contrib.auth.models import Permission, User
 from django.db.utils import IntegrityError
 from django.test.client import Client
 from django.urls import reverse
@@ -560,6 +560,7 @@ class ContainerQATest(FindInit, TestCase):
 
     def setUp(self):
         self.get_default_user()
+        self.alt_username, self.alt_password, self.alt_user = create_user()
         self.main_warehouse = models.Warehouse.objects.create(
             name="Main", warehouse_type=models.WarehouseType.objects.all()[0]
         )
@@ -638,16 +639,16 @@ class ContainerQATest(FindInit, TestCase):
         response = c.get(url)
         self.assertRedirects(response, "/")
         c = Client()
-        c.login(username=self.username, password=self.password)
+        c.login(username=self.alt_username, password=self.alt_password)
         response = c.get(url)
         # change_container permission is needed
-        self.assertEqual(response.status_code, 404)
+        self.assertEqual(response.status_code, 302)
 
         c.logout()
-        self.user.user_permissions.add(
+        self.alt_user.user_permissions.add(
             Permission.objects.get(codename="change_container")
         )
-        c.login(username=self.username, password=self.password)
+        c.login(username=self.alt_username, password=self.alt_password)
         response = c.get(url)
         self.assertEqual(response.status_code, 200)
 
diff --git a/archaeological_warehouse/urls.py b/archaeological_warehouse/urls.py
index a41852b8b..669732a57 100644
--- a/archaeological_warehouse/urls.py
+++ b/archaeological_warehouse/urls.py
@@ -20,19 +20,20 @@
 from django.conf.urls import url
 from django.urls import path
 
-from ishtar_common.utils import check_rights
+from ishtar_common.utils import check_permissions
 
 from archaeological_warehouse import models, views, views_api
 
-# be careful: each check_rights must be relevant with ishtar_menu
+# be careful: each check_permissions must be relevant with ishtar_menu
 
 # forms
 urlpatterns = [
     url(
         r"warehouse_packaging/(?P<step>.+)?$",  # AFAC
-        check_rights(["change_find", "change_own_find"])(
-            views.warehouse_packaging_wizard
-        ),
+        check_permissions(
+            ["archaeological_finds.change_find",
+             "archaeological_finds.change_own_find"]
+        )(views.warehouse_packaging_wizard),
         name="warehouse_packaging",
     ),
     url(
@@ -105,19 +106,24 @@ urlpatterns = [
     ),
     url(
         r"^warehouse_search/(?P<step>.+)?$",
-        check_rights(["view_warehouse", "view_own_warehouse"])(
-            views.warehouse_search_wizard
-        ),
+        check_permissions(
+            ["archaeological_warehouse.view_warehouse",
+             "archaeological_warehouse.view_own_warehouse"]
+        )(views.warehouse_search_wizard),
         name="warehouse_search",
     ),
     url(
         r"^warehouse_creation/(?P<step>.+)?$",
-        check_rights(["add_warehouse"])(views.warehouse_creation_wizard),
+        check_permissions(
+            ["archaeological_warehouse.add_warehouse"]
+        )(views.warehouse_creation_wizard),
         name="warehouse_creation",
     ),
     url(
         r"^warehouse_modification/(?P<step>.+)?$",
-        check_rights(["change_warehouse"])(views.warehouse_modification_wizard),
+        check_permissions(
+            ["archaeological_warehouse.change_warehouse"]
+        )(views.warehouse_modification_wizard),
         name="warehouse_modification",
     ),
     url(
@@ -127,7 +133,9 @@ urlpatterns = [
     ),
     url(
         r"^warehouse_deletion/(?P<step>.+)?$",
-        check_rights(["change_warehouse"])(views.warehouse_deletion_wizard),
+        check_permissions(
+            ["archaeological_warehouse.change_warehouse"]
+        )(views.warehouse_deletion_wizard),
         name="warehouse_deletion",
     ),
     url(
@@ -143,49 +151,51 @@ urlpatterns = [
     ),
     url(
         r"^warehouse-qa-bulk-update/(?P<pks>[0-9-]+)?/$",
-        check_rights([
-            "change_warehouse",
-            "change_own_warehouse"
-        ])(
-            views.QAWarehouseForm.as_view()
-        ),
+        check_permissions([
+            "archaeological_warehouse.change_warehouse",
+            "archaeological_warehouse.change_own_warehouse"
+        ])(views.QAWarehouseForm.as_view()),
         name="warehouse-qa-bulk-update",
     ),
     url(
         r"^warehouse-qa-bulk-update/(?P<pks>[0-9-]+)?/confirm/$",
-        check_rights([
-            "change_warehouse",
-            "change_own_warehouse",
-        ])(
-            views.QAWarehouseForm.as_view()
-        ),
+        check_permissions([
+            "archaeological_warehouse.change_warehouse",
+            "archaeological_warehouse.change_own_warehouse",
+        ])(views.QAWarehouseForm.as_view()),
         name="warehouse-qa-bulk-update-confirm",
         kwargs={"confirm": True},
     ),
     url(
         r"^container-add-treatment/(?P<pk>[0-9-]+)/$",
-        check_rights(["change_find", "change_own_find"])(views.container_treatment_add),
+        check_permissions(
+            ["archaeological_finds.change_find",
+             "archaeological_finds.change_own_find"]
+        )(views.container_treatment_add),
         name="container-add-treatment",
     ),
     url(
         r"^container_search/(?P<step>.+)?$",
-        check_rights(["view_container", "view_own_container"])(
-            views.container_search_wizard
-        ),
+        check_permissions(
+            ["archaeological_warehouse.view_container",
+             "archaeological_warehouse.view_own_container"]
+        )(views.container_search_wizard),
         name="container_search",
     ),
     url(
         r"^container_creation/(?P<step>.+)?$",
-        check_rights(["add_container", "add_own_container"])(
-            views.container_creation_wizard
-        ),
+        check_permissions(
+            ["archaeological_warehouse.add_container",
+             "archaeological_warehouse.add_own_container"]
+        )(views.container_creation_wizard),
         name="container_creation",
     ),
     url(
         r"^container_modification/(?P<step>.+)?$",
-        check_rights(["change_container", "change_own_container"])(
-            views.container_modification_wizard
-        ),
+        check_permissions(
+            ["archaeological_warehouse.change_container",
+             "archaeological_warehouse.change_own_container"]
+        )(views.container_modification_wizard),
         name="container_modification",
     ),
     url(
@@ -195,9 +205,10 @@ urlpatterns = [
     ),
     url(
         r"^container_deletion/(?P<step>.+)?$",
-        check_rights(["change_container", "change_own_container"])(
-            views.container_deletion_wizard
-        ),
+        check_permissions(
+            ["archaeological_warehouse.change_container",
+             "archaeological_warehouse.change_own_container"]
+        )(views.container_deletion_wizard),
         name="container_deletion",
     ),
     url(
@@ -207,24 +218,27 @@ urlpatterns = [
     ),
     url(
         r"^container-qa-bulk-update/(?P<pks>[0-9-]+)?/$",
-        check_rights(["change_container", "change_own_container"])(
-            views.QAContainerForm.as_view()
-        ),
+        check_permissions(
+            ["archaeological_warehouse.change_container",
+             "archaeological_warehouse.change_own_container"]
+        )(views.QAContainerForm.as_view()),
         name="container-qa-bulk-update",
     ),
     url(
         r"^container-qa-bulk-update/(?P<pks>[0-9-]+)?/confirm/$",
-        check_rights(["change_container", "change_own_container"])(
-            views.QAContainerForm.as_view()
-        ),
+        check_permissions(
+            ["archaeological_warehouse.change_container",
+             "archaeological_warehouse.change_own_container"]
+        )(views.QAContainerForm.as_view()),
         name="container-qa-bulk-update-confirm",
         kwargs={"confirm": True},
     ),
     url(
         r"^container-qa-move/(?P<pks>[0-9-]+)?/$",
-        check_rights(["change_container", "change_own_container"])(
-            views.QAContainerMoveForm.as_view()
-        ),
+        check_permissions(
+            ["archaeological_warehouse.change_container",
+             "archaeological_warehouse.change_own_container"]
+        )(views.QAContainerMoveForm.as_view()),
         name="container-qa-move",
     ),
     url(
diff --git a/archaeological_warehouse/views.py b/archaeological_warehouse/views.py
index 4b3f494d8..416849c9a 100644
--- a/archaeological_warehouse/views.py
+++ b/archaeological_warehouse/views.py
@@ -95,10 +95,13 @@ new_container = new_qa_item(
 
 
 def autocomplete_warehouse(request):
-    if not request.user.has_perm(
-        "ishtar_common.view_warehouse", models.Warehouse
-    ) and not request.user.has_perm(
-        "ishtar_common.view_own_warehouse", models.Warehouse
+    ishtaruser = getattr(request.user, "ishtaruser", None)
+    if not ishtaruser:
+        return HttpResponse(content_type="text/plain")
+    if not ishtaruser.has_permission(
+        "archaeological_warehouse.view_warehouse"
+    ) and not ishtaruser.has_permission(
+        "archaeological_warehouse.view_own_warehouse", models.Warehouse
     ):
         return HttpResponse(content_type="text/plain")
     if not request.GET.get("term"):
@@ -106,7 +109,8 @@ def autocomplete_warehouse(request):
     q = request.GET.get("term")
     query = Q()
     for q in q.split(" "):
-        extra = Q(name__icontains=q) | Q(slug__icontains=q) | Q(warehouse_type__label__icontains=q)
+        extra = Q(name__icontains=q) | Q(slug__icontains=q) | \
+            Q(warehouse_type__label__icontains=q)
         query = query & extra
     limit = 15
     warehouses = models.Warehouse.objects.filter(query)[:limit]
@@ -117,10 +121,13 @@ def autocomplete_warehouse(request):
 
 
 def autocomplete_container(request, warehouse_id=None):
-    if not request.user.has_perm(
-        "ishtar_common.view_warehouse", models.Warehouse
-    ) and not request.user.has_perm(
-        "ishtar_common.view_own_warehouse", models.Warehouse
+    ishtaruser = getattr(request.user, "ishtaruser", None)
+    if not ishtaruser:
+        return HttpResponse(content_type="text/plain")
+    if not ishtaruser.has_permission(
+        "archaeological_warehouse.view_container"
+    ) and not ishtaruser.has_permission(
+        "archaeological_warehouse.view_own_container"
     ):
         return HttpResponse(content_type="text/plain")
     if not request.GET.get("term"):