From 8d2616d41d068a1cf501c88e789cccf5b70dbd9f Mon Sep 17 00:00:00 2001
From: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Mon, 11 Oct 2021 18:10:42 +0200
Subject: [PATCH 8/8] apply soft-deletion logic when retrieving roleparenting
 objects (#57500)

---
 src/authentic2/custom_user/models.py          |  7 +-
 .../management/commands/check-and-repair.py   |  4 +-
 src/authentic2/manager/resources.py           |  3 +-
 src/authentic2/manager/role_views.py          |  4 +-
 src/authentic2/manager/tables.py              |  4 +-
 src/authentic2/manager/user_views.py          |  2 +-
 src/django_rbac/managers.py                   | 18 ++++-
 src/django_rbac/models.py                     |  6 +-
 tests_rbac/test_rbac.py                       | 66 ++++++++++++++++++-
 9 files changed, 98 insertions(+), 16 deletions(-)

diff --git a/src/authentic2/custom_user/models.py b/src/authentic2/custom_user/models.py
index 7de79fa0..68063970 100644
--- a/src/authentic2/custom_user/models.py
+++ b/src/authentic2/custom_user/models.py
@@ -219,10 +219,13 @@ class User(AbstractBaseUser, PermissionMixin):
 
     def roles_and_parents(self):
         qs1 = self.roles.all()
-        qs2 = qs1.model.objects.filter(child_relation__child__in=qs1)
+        qs2 = qs1.model.objects.filter(
+            child_relation__deleted__isnull=True,
+            child_relation__child__in=qs1,
+        )
         qs = (qs1 | qs2).order_by('name').distinct()
         RoleParenting = get_role_parenting_model()
-        rp_qs = RoleParenting.objects.filter(child__in=qs1)
+        rp_qs = RoleParenting.active_objects.filter(child__in=qs1)
         qs = qs.prefetch_related(models.Prefetch('child_relation', queryset=rp_qs), 'child_relation__parent')
         qs = qs.prefetch_related(
             models.Prefetch('members', queryset=self.__class__.objects.filter(pk=self.pk), to_attr='member')
diff --git a/src/authentic2/management/commands/check-and-repair.py b/src/authentic2/management/commands/check-and-repair.py
index 45770e13..028aacc0 100644
--- a/src/authentic2/management/commands/check-and-repair.py
+++ b/src/authentic2/management/commands/check-and-repair.py
@@ -388,7 +388,9 @@ class Command(BaseCommand):
                     direct_members = manager_role.members.all()
                     direct_members_count = direct_members.count()
                     direct_children = Role.objects.filter(
-                        parent_relation__parent=manager_role, parent_relation__direct=True
+                        parent_relation__deleted__isnull=True,
+                        parent_relation__parent=manager_role,
+                        parent_relation__direct=True,
                     )
                     direct_children_count = direct_children.count()
                     show = members_count or self.verbosity > 1
diff --git a/src/authentic2/manager/resources.py b/src/authentic2/manager/resources.py
index 3885a4bd..d101dc58 100644
--- a/src/authentic2/manager/resources.py
+++ b/src/authentic2/manager/resources.py
@@ -40,7 +40,8 @@ class UserResource(ModelResource):
         for role in instance.roles.all():
             result.add(role)
             for pr in role.parent_relation.all():
-                result.add(pr.parent)
+                if pr.deleted is None:
+                    result.add(pr.parent)
         return ', '.join(str(x) for x in result)
 
     class Meta:
diff --git a/src/authentic2/manager/role_views.py b/src/authentic2/manager/role_views.py
index f0beb15f..793c3f8b 100644
--- a/src/authentic2/manager/role_views.py
+++ b/src/authentic2/manager/role_views.py
@@ -396,7 +396,7 @@ class RoleChildrenView(RoleViewMixin, views.HideOUColumnMixin, views.BaseSubTabl
             )
         )
         RoleParenting = get_role_parenting_model()
-        rp_qs = RoleParenting.objects.filter(parent__in=children).annotate(name=F('parent__name'))
+        rp_qs = RoleParenting.active_objects.filter(parent__in=children).annotate(name=F('parent__name'))
         qs = qs.prefetch_related(Prefetch('parent_relation', queryset=rp_qs, to_attr='via'))
         return qs
 
@@ -461,7 +461,7 @@ class RoleParentsView(RoleViewMixin, views.HideOUColumnMixin, views.BaseSubTable
             )
         )
         RoleParenting = get_role_parenting_model()
-        rp_qs = RoleParenting.objects.filter(child__in=parents).annotate(name=F('child__name'))
+        rp_qs = RoleParenting.active_objects.filter(child__in=parents).annotate(name=F('child__name'))
         qs = qs.prefetch_related(Prefetch('child_relation', queryset=rp_qs, to_attr='via'))
         return qs
 
diff --git a/src/authentic2/manager/tables.py b/src/authentic2/manager/tables.py
index b0a123d5..7b0f968d 100644
--- a/src/authentic2/manager/tables.py
+++ b/src/authentic2/manager/tables.py
@@ -182,8 +182,8 @@ class UserRolesTable(tables.Table):
     )
     ou = tables.Column()
     via = tables.TemplateColumn(
-        '{% if not record.member %}{% for rel in record.child_relation.all %}{{ rel.child }} {% if not'
-        ' forloop.last %}, {% endif %}{% endfor %}{% endif %}',
+        '{% if not record.member %}{% for rel in record.child_relation.all %}{% if not rel.deleted %}'
+        '{{ rel.child }} {% if not forloop.last %}, {% endif %}{% endif %}{% endfor %}{% endif %}',
         verbose_name=_('Inherited from'),
         orderable=False,
     )
diff --git a/src/authentic2/manager/user_views.py b/src/authentic2/manager/user_views.py
index 5e364981..3faddb35 100644
--- a/src/authentic2/manager/user_views.py
+++ b/src/authentic2/manager/user_views.py
@@ -643,7 +643,7 @@ class UserRolesView(HideOUColumnMixin, BaseSubTableView):
             User = get_user_model()
             Role = get_role_model()
             RoleParenting = get_role_parenting_model()
-            rp_qs = RoleParenting.objects.filter(child__in=roles)
+            rp_qs = RoleParenting.active_objects.filter(child__in=roles)
             qs = Role.objects.all()
             qs = qs.prefetch_related(models.Prefetch('child_relation', queryset=rp_qs, to_attr='via'))
             qs = qs.prefetch_related(
diff --git a/src/django_rbac/managers.py b/src/django_rbac/managers.py
index 2e6063e3..e3700201 100644
--- a/src/django_rbac/managers.py
+++ b/src/django_rbac/managers.py
@@ -109,7 +109,10 @@ class RoleQuerySet(query.QuerySet):
         return self.filter(members=user).parents().distinct()
 
     def parents(self, include_self=True, annotate=False):
-        qs = self.model.objects.filter(child_relation__child__in=self)
+        qs = self.model.objects.filter(
+            child_relation__deleted__isnull=True,
+            child_relation__child__in=self,
+        )
         if include_self:
             qs = self | qs
         qs = qs.distinct()
@@ -118,7 +121,10 @@ class RoleQuerySet(query.QuerySet):
         return qs
 
     def children(self, include_self=True, annotate=False):
-        qs = self.model.objects.filter(parent_relation__parent__in=self)
+        qs = self.model.objects.filter(
+            parent_relation__deleted__isnull=True,
+            parent_relation__parent__in=self,
+        )
         if include_self:
             qs = self | qs
         qs = qs.distinct()
@@ -130,7 +136,13 @@ class RoleQuerySet(query.QuerySet):
         User = get_user_model()
         prefetch = Prefetch('roles', queryset=self, to_attr='direct')
         return (
-            User.objects.filter(Q(roles__in=self) | Q(roles__parent_relation__parent__in=self))
+            User.objects.filter(
+                Q(roles__in=self)
+                | (
+                    Q(roles__parent_relation__parent__in=self)
+                    & Q(roles__parent_relation__deleted__isnull=True)
+                )
+            )
             .distinct()
             .prefetch_related(prefetch)
         )
diff --git a/src/django_rbac/models.py b/src/django_rbac/models.py
index 37e1c138..74e62a17 100644
--- a/src/django_rbac/models.py
+++ b/src/django_rbac/models.py
@@ -217,8 +217,12 @@ class RoleAbstractBase(AbstractOrganizationalUnitScopedBase, AbstractBase):
     def all_members(self):
         User = get_user_model()
         prefetch = Prefetch('roles', queryset=self.__class__.objects.filter(pk=self.pk), to_attr='direct')
+
         return (
-            User.objects.filter(Q(roles=self) | Q(roles__parent_relation__parent=self))
+            User.objects.filter(
+                Q(roles=self)
+                | Q(roles__parent_relation__parent=self) & Q(roles__parent_relation__deleted__isnull=True)
+            )
             .distinct()
             .prefetch_related(prefetch)
         )
diff --git a/tests_rbac/test_rbac.py b/tests_rbac/test_rbac.py
index a6f38b95..05a295ea 100644
--- a/tests_rbac/test_rbac.py
+++ b/tests_rbac/test_rbac.py
@@ -66,9 +66,19 @@ def test_role_parenting(db):
         assert role.parents().count() == i + 1
         assert role.children(False).count() == 6 - i - 1
         assert role.parents(False).count() == i
-    RoleParenting.objects.filter(parent=roles[2], child=roles[3], direct=True).delete()
-    assert RoleParenting.objects.filter(direct=True).count() == 4
-    assert RoleParenting.objects.filter(direct=False).count() == 2
+    RoleParenting.objects.soft_delete(roles[2], roles[3], direct=True)
+    assert (
+        RoleParenting.active_objects.filter(
+            direct=True,
+        ).count()
+        == 4
+    )
+    assert (
+        RoleParenting.active_objects.filter(
+            direct=False,
+        ).count()
+        == 2
+    )
     # test that it works with cycles
     RoleParenting.objects.create(parent=roles[2], child=roles[3])
     RoleParenting.objects.create(parent=roles[5], child=roles[0])
@@ -77,6 +87,56 @@ def test_role_parenting(db):
         assert role.parents().count() == 6
 
 
+def test_role_parenting_soft_delete_children(db):
+    OrganizationalUnit = utils.get_ou_model()
+    Role = utils.get_role_model()
+    RoleParenting = utils.get_role_parenting_model()
+
+    ou = OrganizationalUnit.objects.create(name='ou')
+    roles = []
+    for i in range(10):
+        roles.append(Role.objects.create(name='r%d' % i, ou=ou))
+    assert not len(RoleParenting.objects.all())
+
+    rps = []
+    for i in range(5):
+        rps.append(RoleParenting.objects.create(parent=roles[9 - i], child=roles[i]))
+    assert len(RoleParenting.objects.all()) == 5
+    for i in range(5):
+        roles[9 - i].remove_child(roles[i])
+        assert len(RoleParenting.objects.all()) == 5
+        assert len(RoleParenting.active_objects.all()) == 4 - i
+    for i in range(5):
+        roles[9 - i].add_child(roles[i])
+        assert len(RoleParenting.objects.all()) == 5
+        assert len(RoleParenting.active_objects.all()) == i + 1
+
+
+def test_role_parenting_soft_delete_parents(db):
+    OrganizationalUnit = utils.get_ou_model()
+    Role = utils.get_role_model()
+    RoleParenting = utils.get_role_parenting_model()
+
+    ou = OrganizationalUnit.objects.create(name='ou')
+    roles = []
+    for i in range(10):
+        roles.append(Role.objects.create(name='r%d' % i, ou=ou))
+    assert not len(RoleParenting.objects.all())
+
+    rps = []
+    for i in range(5):
+        rps.append(RoleParenting.objects.create(child=roles[9 - i], parent=roles[i]))
+    assert len(RoleParenting.objects.all()) == 5
+    for i in range(5):
+        roles[9 - i].remove_parent(roles[i])
+        assert len(RoleParenting.objects.all()) == 5
+        assert len(RoleParenting.active_objects.all()) == 4 - i
+    for i in range(5):
+        roles[9 - i].add_parent(roles[i])
+        assert len(RoleParenting.objects.all()) == 5
+        assert len(RoleParenting.active_objects.all()) == i + 1
+
+
 SIZE = 1000
 SPAN = 50
 
-- 
2.33.0

