🐛 imports list: fix permissions check

3 files changed, 57 insertions, 47 deletions

diff --git a/ishtar_common/models_imports.py b/ishtar_common/models_imports.py
index e09ca2502..3e8914d8d 100644
--- a/ishtar_common/models_imports.py
+++ b/ishtar_common/models_imports.py
@@ -235,9 +235,10 @@ class ImporterType(models.Model):
     def __str__(self):
         return self.name
 
-    @classmethod
-    def is_own(cls, ishtar_user):
-        return bool(cls.objects.filter(users__pk=ishtar_user.pk).count())
+    def is_own(self, ishtar_user):
+        return bool(
+            self.__class__.objects.filter(pk=self.pk, users__pk=ishtar_user.pk).count()
+        )
 
     @property
     def type_label(self):
@@ -1457,22 +1458,33 @@ class BaseImport(models.Model, OwnPerms, SheetItem):
     def get_permissions_for_actions(cls, user):
         if not hasattr(user, "ishtaruser") or not user.ishtaruser:
             return False, False, False, False
-        can_edit_all, can_delete_all = False, False
-        can_edit_own, can_delete_own = False, False
+        permissions = {
+            "can_view_own": False,
+            "can_edit_own": False,
+            "can_delete_own": False,
+            "can_edit_all": False,
+            "can_view_all": False,
+            "can_delete_all": False,
+        }
         ishtaruser = user.ishtaruser
         if ishtaruser.has_permission("ishtaradmin"):
-            can_edit_all = True
-            can_delete_all = True
-        else:
-            if ishtaruser.has_permission("ishtar_common.change_import"):
-                can_edit_all = True
-            elif ishtaruser.has_permission("ishtar_common.change_own_import"):
-                can_edit_own = True
-            if ishtaruser.has_permission("ishtar_common.delete_import"):
-                can_delete_all = True
-            elif ishtaruser.has_permission("ishtar_common.delete_own_import"):
-                can_delete_own = True
-        return can_edit_all, can_delete_all, can_edit_own, can_delete_own
+            permissions["can_view_all"] = True
+            permissions["can_edit_all"] = True
+            permissions["can_delete_all"] = True
+            return permissions
+        if ishtaruser.has_permission("ishtar_common.view_import"):
+            permissions["can_view_all"] = True
+        elif ishtaruser.has_permission("ishtar_common.view_own_import"):
+            permissions["can_view_own"] = True
+        if ishtaruser.has_permission("ishtar_common.change_import"):
+            permissions["can_edit_all"] = True
+        elif ishtaruser.has_permission("ishtar_common.change_own_import"):
+            permissions["can_edit_own"] = True
+        if ishtaruser.has_permission("ishtar_common.delete_import"):
+            permissions["can_delete_all"] = True
+        elif ishtaruser.has_permission("ishtar_common.delete_own_import"):
+            permissions["can_delete_own"] = True
+        return permissions
 
     @classmethod
     def query_can_access(cls, user, perm="ishtar_common.view_import"):
diff --git a/ishtar_common/urls.py b/ishtar_common/urls.py
index 482e647f7..b9eb88c2b 100644
--- a/ishtar_common/urls.py
+++ b/ishtar_common/urls.py
@@ -260,32 +260,32 @@ urlpatterns = [
     url(
         r"^import-list/$",
         check_permissions(
-            ["ishtar_common.view_import", "ishtar_common.change_import",
-             "ishtar_common.change_own_import"]
+            ["ishtar_common.view_import", "ishtar_common.view_own_import",
+             "ishtar_common.change_import", "ishtar_common.change_own_import"]
         )(views.ImportListView.as_view()),
         name="current_imports",
     ),
     url(
         r"^import-list-table/$",
         check_permissions(
-            ["ishtar_common.view_import", "ishtar_common.change_import",
-             "ishtar_common.change_own_import"]
+            ["ishtar_common.view_import", "ishtar_common.view_own_import",
+             "ishtar_common.change_import", "ishtar_common.change_own_import"]
         )(views.ImportListTableView.as_view()),
         name="current_imports_table",
     ),
     url(
        r"^import-get-status/$",
        check_permissions(
-           ["ishtar_common.view_import", "ishtar_common.change_import",
-            "ishtar_common.change_own_import"]
+           ["ishtar_common.view_import", "ishtar_common.view_own_import",
+            "ishtar_common.change_import", "ishtar_common.change_own_import"]
        )(views.import_get_status),
        name="import_get_status",
     ),
     url(
         r"^import-list-old/$",
         check_permissions(
-            ["ishtar_common.view_import", "ishtar_common.change_import",
-             "ishtar_common.change_own_import"]
+            ["ishtar_common.view_import", "ishtar_common.view_own_import",
+             "ishtar_common.change_import", "ishtar_common.change_own_import"]
         )(views.ImportOldListView.as_view()),
         name="old_imports",
     ),
diff --git a/ishtar_common/views.py b/ishtar_common/views.py
index aa47040aa..a4242aa3b 100644
--- a/ishtar_common/views.py
+++ b/ishtar_common/views.py
@@ -1788,20 +1788,22 @@ class ImportPreFormView(IshtarMixin, LoginRequiredMixin, FormView):
         return HttpResponseRedirect(self.get_success_url())
 
 
-def get_permissions_for_actions(
-        user, imprt, owns, can_edit_all, can_delete_all, can_edit_own, can_delete_own):
-    can_edit, can_delete = False, False
+def get_permissions_for_actions(user, imprt, owns, permissions):
+    can_view, can_edit, can_delete = False, False, False
     is_own = None
-    if can_edit_own or can_delete_own:  # need to check owner
+    if permissions["can_edit_own"] or permissions["can_delete_own"] \
+            or permissions["can_view_own"]:  # need to check owner
         if imprt.importer_type_id not in owns:
             # "is_own" only query once by importer type
             owns[imprt.importer_type.pk] = imprt.importer_type.is_own(user.ishtaruser)
         is_own = owns[imprt.importer_type_id]
-    if can_edit_all or (can_edit_own and is_own):
+    if permissions["can_view_all"] or (permissions["can_view_own"] and is_own):
+        can_view = True
+    if permissions["can_edit_all"] or (permissions["can_edit_own"] and is_own):
         can_edit = True
-    if can_delete_all or (can_delete_own and is_own):
+    if permissions["can_delete_all"] or (permissions["can_delete_own"] and is_own):
         can_delete = True
-    return can_edit, can_delete
+    return can_view, can_edit, can_delete
 
 
 class ImportListView(IshtarMixin, LoginRequiredMixin, ListView):
@@ -1839,15 +1841,15 @@ class ImportListView(IshtarMixin, LoginRequiredMixin, ListView):
                 key=lambda x: (x.end_date or x.creation_date)
             )
         ))
-        can_edit_all, can_delete_all, can_edit_own, can_delete_own = \
-            models.Import.get_permissions_for_actions(user)
+        permissions = models.Import.get_permissions_for_actions(user)
         imports = []
         owns = {}
         for imprt in values:
-            can_edit, can_delete = get_permissions_for_actions(
-                user, imprt, owns, can_edit_all,
-                can_delete_all, can_edit_own, can_delete_own
+            can_view, can_edit, can_delete = get_permissions_for_actions(
+                user, imprt, owns, permissions
             )
+            if not can_view:
+                continue
             imprt.action_list = imprt.get_actions(
                 can_edit=can_edit, can_delete=can_delete
             )
@@ -1863,8 +1865,7 @@ class ImportListView(IshtarMixin, LoginRequiredMixin, ListView):
         return imports
 
     def post(self, request, *args, **kwargs):
-        can_edit_all, can_delete_all, can_edit_own, can_delete_own = \
-            models.Import.get_permissions_for_actions(request.user)
+        permissions = models.Import.get_permissions_for_actions(request.user)
         owns = {}
         for field in request.POST:
             if not field.startswith("import-action-") or not request.POST[field]:
@@ -1878,9 +1879,8 @@ class ImportListView(IshtarMixin, LoginRequiredMixin, ListView):
                 imprt = model.objects.get(pk=int(field.split("-")[-1]))
             except (models.Import.DoesNotExist, ValueError):
                 continue
-            can_edit, can_delete = get_permissions_for_actions(
-                request.user, imprt, owns, can_edit_all,
-                can_delete_all, can_edit_own, can_delete_own
+            can_view, can_edit, can_delete = get_permissions_for_actions(
+                request.user, imprt, owns, permissions
             )
             action = request.POST[field]
             if can_delete and action == "D":
@@ -2456,11 +2456,9 @@ def import_get_status(request, current_right=None):
                 "number_of_line": item.number_of_line,
                 "progress_percent": item.progress_percent,
             })
-        can_edit_all, can_delete_all, can_edit_own, can_delete_own = \
-            models.Import.get_permissions_for_actions(request.user)
-        can_edit, can_delete = get_permissions_for_actions(
-            request.user, item, {}, can_edit_all,
-            can_delete_all, can_edit_own, can_delete_own
+        permissions = models.Import.get_permissions_for_actions(request.user)
+        can_view, can_edit, can_delete = get_permissions_for_actions(
+            request.user, item, {}, permissions
         )
         item_dct["actions"] = [
             (key, str(lbl))