✨ permissions refactoring: manage deletion permissions - ♻ refactoring "can_do"

13 files changed, 154 insertions, 73 deletions

diff --git a/archaeological_context_records/models.py b/archaeological_context_records/models.py
index 21cc507c1..a3219e76a 100644
--- a/archaeological_context_records/models.py
+++ b/archaeological_context_records/models.py
@@ -1176,11 +1176,9 @@ class ContextRecord(
         actions = super().get_extra_actions(request)
         is_locked = hasattr(self, "is_locked") and self.is_locked(request.user)
 
-        can_edit_cr = self.can_do(
-            request, "archaeological_context_records.change_contextrecord"
-        )
+        can_edit_cr = self.can_change(request)
         profile = get_current_profile()
-        can_add_geo = profile.mapping and \
+        can_add_geo = can_edit_cr and profile.mapping and \
             self.can_do(request, "ishtar_common.add_geovectordata")
         if can_add_geo:
             actions.append(self.get_add_geo_action())
diff --git a/archaeological_files/ishtar_menu.py b/archaeological_files/ishtar_menu.py
index 5541fd1d3..26a1ca8ab 100644
--- a/archaeological_files/ishtar_menu.py
+++ b/archaeological_files/ishtar_menu.py
@@ -111,7 +111,7 @@ MENU_SECTIONS = [
                             _("Deletion"),
                             model=AdministrativeAct,
                             access_controls=[
-                                "archaeological_operations.change_administrativeact"
+                                "archaeological_operations.delete_administrativeact"
                             ],
                         ),
                     ],
diff --git a/archaeological_files/urls.py b/archaeological_files/urls.py
index 4c278e0be..d8e473451 100644
--- a/archaeological_files/urls.py
+++ b/archaeological_files/urls.py
@@ -56,7 +56,7 @@ urlpatterns = [
     ),
     url(
         r"file_administrativeactfile_deletion/(?P<step>.+)?$",
-        check_permissions(["archaeological_operations.change_administrativeact"])(
+        check_permissions(["archaeological_operations.delete_administrativeact"])(
             views.file_administrativeactfile_deletion_wizard
         ),
         name="file_administrativeactfile_deletion",
diff --git a/archaeological_finds/urls.py b/archaeological_finds/urls.py
index c7d8356ce..935deba17 100644
--- a/archaeological_finds/urls.py
+++ b/archaeological_finds/urls.py
@@ -59,8 +59,8 @@ urlpatterns = [
     url(
         r"find_deletion/(?P<step>.+)?$",
         check_permissions(
-            ["archaeological_finds.change_find",
-             "archaeological_finds.change_own_find"]
+            ["archaeological_finds.delete_find",
+             "archaeological_finds.delete_own_find"]
         )(views.find_deletion_wizard),
         name="find_deletion",
     ),
@@ -334,8 +334,8 @@ urlpatterns = [
     url(
         r"^treatment_deletion/(?P<step>.+)?$",
         check_permissions(
-            ["archaeological_finds.change_treatmentfile",
-             "archaeological_finds.change_own_treatmentfile"]
+            ["archaeological_finds.delete_treatmentfile",
+             "archaeological_finds.delete_own_treatmentfile"]
         )(views.treatment_deletion_wizard),
         name="treatment_deletion",
     ),
@@ -372,7 +372,7 @@ urlpatterns = [
     ),
     url(
         r"^treatment_admacttreatment_deletion/(?P<step>.+)?$",
-        check_permissions(["archaeological_operations.change_administrativeact"])(
+        check_permissions(["archaeological_operations.delete_administrativeact"])(
             views.treatment_admacttreatment_deletion_wizard
         ),
         name="treatment_admacttreatment_deletion",
@@ -415,7 +415,7 @@ urlpatterns = [
     ),
     url(
         r"^treatmentfle_admacttreatmentfle_deletion/(?P<step>.+)?$",
-        check_permissions(["archaeological_operations.change_administrativeact"])(
+        check_permissions(["archaeological_operations.delete_administrativeact"])(
             views.treatmentfile_admacttreatmentfile_deletion_wizard
         ),
         name="treatmentfle_admacttreatmentfle_deletion",
@@ -457,8 +457,8 @@ urlpatterns = [
     url(
         r"^treatmentfle_deletion/(?P<step>.+)?$",
         check_permissions(
-            ["archaeological_finds.change_find",
-             "archaeological_finds.change_own_find"]
+            ["archaeological_finds.delete_find",
+             "archaeological_finds.delete_own_find"]
             )(views.treatmentfile_deletion_wizard),
         name="treatmentfile_deletion",
     ),
diff --git a/archaeological_operations/ishtar_menu.py b/archaeological_operations/ishtar_menu.py
index 31d7ade34..8dd1049e8 100644
--- a/archaeological_operations/ishtar_menu.py
+++ b/archaeological_operations/ishtar_menu.py
@@ -73,8 +73,8 @@ MENU_SECTIONS = [
                     _("Deletion"),
                     model=models.Operation,
                     access_controls=[
-                        "archaeological_operations.change_operation",
-                        "archaeological_operations.change_own_operation"
+                        "archaeological_operations.delete_operation",
+                        "archaeological_operations.delete_own_operation"
                     ],
                 ),
                 SectionItem(
@@ -111,7 +111,7 @@ MENU_SECTIONS = [
                             _("Deletion"),
                             model=models.AdministrativeAct,
                             access_controls=[
-                                "archaeological_operations.change_administrativeact"
+                                "archaeological_operations.delete_administrativeact"
                             ],
                         ),
                     ],
@@ -179,7 +179,8 @@ MENU_SECTIONS = [
                     _("Deletion"),
                     model=models.ArchaeologicalSite,
                     access_controls=[
-                        "archaeological_operations.change_archaeologicalsite"
+                        "archaeological_operations.delete_archaeologicalsite",
+                        "archaeological_operations.delete_own_archaeologicalsite"
                     ],
                 ),
             ],
diff --git a/archaeological_operations/views.py b/archaeological_operations/views.py
index eccf0e0b6..a93ba80e3 100644
--- a/archaeological_operations/views.py
+++ b/archaeological_operations/views.py
@@ -794,7 +794,8 @@ def site_add_operation(request, pks, current_right=None):
         if not q.count():
             raise Http404()
         site = q.all()[0]
-        if not site.can_do(request, "view_archaeologicalsite") \
+        if not site.can_do(request, "view", app="archaeological_operations",
+                           model_name="archaeologicalsite") \
                 or site.operations.count():
             raise Http404()
     # operation add permission checked on view call
@@ -813,7 +814,8 @@ def site_add_top_operation(request, pks, current_right=None):
     if not q.count():
         raise Http404()
     site = q.all()[0]
-    if not site.can_do(request, "view_archaeologicalsite") \
+    if not site.can_do(request, "view", app="archaeological_operations",
+                       model_name="archaeologicalsite") \
             or not site.operations.count():
         raise Http404()
     # operation add permission checked on view call
diff --git a/archaeological_warehouse/ishtar_menu.py b/archaeological_warehouse/ishtar_menu.py
index c0b8d658b..9b7cb98cb 100644
--- a/archaeological_warehouse/ishtar_menu.py
+++ b/archaeological_warehouse/ishtar_menu.py
@@ -58,6 +58,7 @@ MENU_SECTIONS = [
                     model=models.Warehouse,
                     access_controls=[
                         "archaeological_warehouse.change_warehouse",
+                        "archaeological_warehouse.change_own_warehouse"
                     ],
                 ),
                 MenuItem(
@@ -65,7 +66,8 @@ MENU_SECTIONS = [
                     _("Deletion"),
                     model=models.Warehouse,
                     access_controls=[
-                        "archaeological_warehouse.change_warehouse",
+                        "archaeological_warehouse.delete_warehouse",
+                        "archaeological_warehouse.delete_own_warehouse"
                     ],
                 ),
             ],
@@ -123,14 +125,15 @@ MENU_SECTIONS = [
                     _("Deletion"),
                     model=models.Warehouse,
                     access_controls=[
-                        "archaeological_warehouse.change_container",
-                        "archaeological_warehouse.change_own_container",
+                        "archaeological_warehouse.delete_container",
+                        "archaeological_warehouse.delete_own_container",
                     ],
                 ),
             ],
         ),
     ),
 ]
+
 """
     MenuItem('warehouse_inventory', _("Inventory"),
         model=models.Warehouse,
diff --git a/ishtar_common/ishtar_menu.py b/ishtar_common/ishtar_menu.py
index aa8ed3b8f..8cf047d51 100644
--- a/ishtar_common/ishtar_menu.py
+++ b/ishtar_common/ishtar_menu.py
@@ -72,8 +72,8 @@ MENU_SECTIONS = [
                             "person_deletion",
                             _("Deletion"),
                             model=models.Person,
-                            access_controls=["ishtar_common.change_person",
-                                             "ishtar_common.change_own_person"],
+                            access_controls=["ishtar_common.delete_person",
+                                             "ishtar_common.delete_own_person"],
                         ),
                     ],
                 ),
@@ -147,8 +147,8 @@ MENU_SECTIONS = [
                             _("Deletion"),
                             model=models.Organization,
                             access_controls=[
-                                "ishtar_common.change_organization",
-                                "ishtar_common.change_own_organization",
+                                "ishtar_common.delete_organization",
+                                "ishtar_common.delete_own_organization",
                             ],
                         ),
                     ],
@@ -210,8 +210,8 @@ MENU_SECTIONS = [
                     "document/delete",
                     _("Deletion"),
                     model=models.Document,
-                    access_controls=["ishtar_common.change_document",
-                                     "ishtar_common.change_own_document"],
+                    access_controls=["ishtar_common.delete_document",
+                                     "ishtar_common.delete_own_document"],
                 ),
             ],
         ),
diff --git a/ishtar_common/migrations/0255_migrate_delete_permissions.py b/ishtar_common/migrations/0255_migrate_delete_permissions.py
new file mode 100644
index 000000000..61b63c0df
--- /dev/null
+++ b/ishtar_common/migrations/0255_migrate_delete_permissions.py
@@ -0,0 +1,52 @@
+# Generated by Django 2.2.28 on 2024-11-04 16:52
+
+from django.db import migrations
+
+
+def migrate_permission(apps, __):
+    Permission = apps.get_model("auth", "permission")
+    Group = apps.get_model("auth", "group")
+    ProfileType = apps.get_model("ishtar_common", "profiletype")
+    print()
+    for modif_group in Group.objects.filter(
+            name__endswith="modification/suppression").all():
+        name = modif_group.name.replace("/suppression", "")
+        modif_group.name = name
+        modif_group.save()
+        delete_permissions = []
+        for permission in modif_group.permissions.filter(
+                codename__startswith="change_").all():
+            codename = permission.codename.replace("change_", "delete_")
+            try:
+                delete_permission = Permission.objects.get(
+                    content_type=permission.content_type,
+                    codename=codename
+                )
+                delete_permissions.append(delete_permission)
+                if delete_permission in list(modif_group.permissions.all()):
+                    modif_group.permissions.remove(delete_permission)
+            except Permission.DoesNotExist:
+                print(f"Permission {codename} does not exist")
+
+        if not delete_permissions:
+            continue
+        delete_group = Group.objects.create(
+            name=name.replace("modification", "suppression")
+        )
+        print(f"* New group: {delete_group.name}")
+        for delete_permission in delete_permissions:
+            delete_group.permissions.add(delete_permission)
+        for profile_type in ProfileType.objects.filter(groups__pk=modif_group.pk).all():
+            profile_type.groups.add(delete_group)
+            print(f"\t- profile type {profile_type.label} updated")
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('ishtar_common', '0254_permissionrequests'),
+    ]
+
+    operations = [
+        migrations.RunPython(migrate_permission)
+    ]
diff --git a/ishtar_common/models.py b/ishtar_common/models.py
index bf2cd666a..5b2e3fdbf 100644
--- a/ishtar_common/models.py
+++ b/ishtar_common/models.py
@@ -71,7 +71,7 @@ from django.core.exceptions import (
 )
 from django.core.files.base import ContentFile
 from django.core.files.uploadedfile import SimpleUploadedFile
-from django.db import connection
+from django.db import connection, transaction
 from django.db.models import Q, Max, Count
 from django.db.models.signals import post_save, post_delete, m2m_changed
 from django.db.utils import DatabaseError
@@ -3440,7 +3440,8 @@ class GDPRLog(models.Model):
 
 
 class ProfileType(GeneralType):
-    groups = models.ManyToManyField(Group, verbose_name=_("Groups"), blank=True)
+    groups = models.ManyToManyField(Group, verbose_name=_("Groups"), blank=True,
+                                    related_name="profile_types")
     permission_requests = models.ManyToManyField(
         PermissionRequest, verbose_name=_("Permissions requests"), blank=True,
         related_name="profile_types"
@@ -3652,26 +3653,27 @@ class UserProfile(models.Model):
                     permission_type
                 )
         user_id = ishtar_user.user_ptr.pk
-        object_permissions = []
         item_ids = list(set(item_ids))
         permissions = list(set(permissions))
-        for permission in permissions:
-            permission_id = permission.pk
-            exclude = list(UserObjectPermission.objects.filter(
-                content_type_id=content_type.pk, permission_id=permission_id,
-                user_id=user_id
-            ).values_list("object_pk", flat=True))
-            object_permissions += [
-                UserObjectPermission(
-                    object_pk=str(item_id),
-                    content_type_id=content_type.pk,
-                    permission_id=permission_id,
+        with transaction.atomic():
+            object_permissions = []
+            for permission in permissions:
+                permission_id = permission.pk
+                exclude = list(UserObjectPermission.objects.filter(
+                    content_type_id=content_type.pk, permission_id=permission_id,
                     user_id=user_id
-                )
-                for item_id in item_ids if str(item_id) not in exclude
-            ]
-        if object_permissions:
-            UserObjectPermission.objects.bulk_create(object_permissions)
+                ).values_list("object_pk", flat=True))
+                object_permissions += [
+                    UserObjectPermission(
+                        object_pk=str(item_id),
+                        content_type_id=content_type.pk,
+                        permission_id=permission_id,
+                        user_id=user_id
+                    )
+                    for item_id in item_ids if str(item_id) not in exclude
+                ]
+            if object_permissions:
+                UserObjectPermission.objects.bulk_create(object_permissions)
 
     def save(
         self,
diff --git a/ishtar_common/templatetags/window_header.py b/ishtar_common/templatetags/window_header.py
index 23d779944..d85ce847b 100644
--- a/ishtar_common/templatetags/window_header.py
+++ b/ishtar_common/templatetags/window_header.py
@@ -33,6 +33,8 @@ def window_nav(context, item, window_id, show_url, modify_url='', histo_url='',
     can_edit = hasattr(item, 'can_edit') and item.can_edit(context['request'])
     if can_edit:
         modify_url_value = modify_url
+    can_delete = hasattr(item, 'can_delete') and item.can_delete(context['request'])
+    if can_delete:
         delete_url = getattr(item, "DELETE_URL", False)
     return {
         'current_user': context['request'].user,
diff --git a/ishtar_common/urls.py b/ishtar_common/urls.py
index 8bb8dd356..a98a34882 100644
--- a/ishtar_common/urls.py
+++ b/ishtar_common/urls.py
@@ -88,7 +88,7 @@ urlpatterns = [
     url(
         r"person_deletion/(?P<step>.+)?$",
         check_permissions(
-            ["ishtar_common.change_person", "ishtar_common.change_own_person"]
+            ["ishtar_common.delete_person", "ishtar_common.delete_own_person"]
         )(views.person_deletion_wizard),
         name="person_deletion",
     ),
@@ -149,7 +149,7 @@ urlpatterns = [
     url(
         r"organization_deletion/(?P<step>.+)?$",
         check_permissions(
-            ["ishtar_common.change_organization", "ishtar_common.change_own_organization"]
+            ["ishtar_common.delete_organization", "ishtar_common.delete_own_organization"]
         )(views.organization_deletion_wizard),
         name="organization_deletion",
     ),
@@ -633,7 +633,7 @@ urlpatterns += [
     url(
         r"document/delete/(?P<step>.+)?$",
         check_permissions(
-            ["ishtar_common.change_document", "ishtar_common.change_own_document"]
+            ["ishtar_common.delete_document", "ishtar_common.delete_own_document"]
         )(views.document_deletion_wizard),
         name="document_deletion",
     ),
@@ -711,8 +711,8 @@ urlpatterns += [
     url(
         r"geo/delete/(?P<pk>\d+)/$",
         check_permissions(
-            ["ishtar_common.change_geovectordata",
-                "ishtar_common.change_own_geovectordata"]
+            ["ishtar_common.delete_geovectordata",
+             "ishtar_common.delete_own_geovectordata"]
         )(views.GeoDeleteView.as_view()),
         name="delete-geo",
     ),
diff --git a/ishtar_common/utils.py b/ishtar_common/utils.py
index 11ff45fa7..c35824906 100644
--- a/ishtar_common/utils.py
+++ b/ishtar_common/utils.py
@@ -422,44 +422,65 @@ class OwnPerms:
         """
         return None  # implement for each object
 
+    def can_add(self, request):
+        meta = self.__class__._meta
+        return self.can_do(
+            request, "add", app=meta.app_label, model_name=meta.model_name
+        )
+
     def can_view(self, request):
         meta = self.__class__._meta
-        perm = f"{meta.app_label}.view_{meta.model_name}"
-        return self.can_do(request, perm)
+        return self.can_do(
+            request, "view", app=meta.app_label, model_name=meta.model_name
+        )
+
+    def can_change(self, request):
+        return self.can_edit(request)
 
     def can_edit(self, request):
-        if not getattr(request.user, "ishtaruser", None):
-            return False
-        ishtaruser = request.user.ishtaruser
         meta = self.__class__._meta
-        perm = f"{meta.app_label}.change_{meta.model_name}"
-        if ishtaruser.has_permission(perm):
-            return True
-        own_perm = f"{meta.app_label}.change_own_{meta.model_name}"
-        if not ishtaruser.has_permission(own_perm):
-            return False
-        return self.is_own(ishtaruser)
+        return self.can_do(
+            request, "change", app=meta.app_label, model_name=meta.model_name
+        )
+
+    def can_delete(self, request):
+        meta = self.__class__._meta
+        return self.can_do(
+            request, "delete", app=meta.app_label, model_name=meta.model_name
+        )
 
-    def can_do(self, request, permission):
+    def can_do(self, request, permission, app=None, model_name=None):
         """
         Check permission availability for the current object.
         :param request: request object
         :param permission: action name eg: "archaelogical_finds.change_find" - "own"
-        variation is checked
+        variation is checked - can provide only simple permission (e.g. "change") if app
+        and model_name are provided
+        :param app: application name (if permission not fully provided)
+        :param model_name: model name (if permission not fully provided)
         :return: boolean
         """
         if not getattr(request.user, "ishtaruser", None):
             return False
 
-        if "_findbasket" in permission:
-            permission = permission.replace("basket", "")
+        if (app and not model_name) or (not app and model_name):
+            return False
+
+        if not app:
+            app, perm = permission.split(".")
+            p = perm.split("_")
+            permission = p[0]
+            model_name = ('_').join(p[1:])
+
+        if model_name == "findbasket":
+            model_name = "find"
+
         ishtaruser = request.user.ishtaruser
+        full_permission = f"{app}.{permission}_{model_name}"
 
-        if ishtaruser.has_permission(permission):
+        if ishtaruser.has_permission(full_permission):
             return True
-        app, perm = permission.split(".")
-        p = perm.split("_")
-        own = f"{app}.{p[0]}_own_{('_').join(p[1:])}"
+        own = f"{app}.{permission}_own_{model_name}"
         try:
             return ishtaruser.has_permission(own, self)
         except WrongAppError: