Project

General

Profile

Bug #57500

Ajouter des champs created et deleted (soft delete) sur RoleParenting

Added by Benjamin Dauvergne 24 days ago. Updated 11 days ago.

Status:
Solution proposée
Priority:
Normal
Category:
-
Target version:
-
Start date:
01 Oct 2021
Due date:
% Done:

0%

Estimated time:
Patch proposed:
Yes
Planning:
No

Description

Cf. #57499

  • created : DateField(auto_now_add=True)
  • deleted : DateField(null=True)

Les suppression d'héritage doivent être revus pour faire un soft-delete et les créations pour supprimer la valeur deleted et rétablir created à now().

Les méthodes pour lister parents/enfants doivent ignorer les relations d'héritage supprimées (il faut faire le tour des utilisations de RoleParenting).


Files

0002-rbac-add-soft-created-deleted-model-methods-57500.patch (1.15 KB) 0002-rbac-add-soft-created-deleted-model-methods-57500.patch Paul Marillonnet, 12 Oct 2021 10:52 AM
0003-rbac-add-manager-soft-deletion-shortcut-filters-5750.patch (1.26 KB) 0003-rbac-add-manager-soft-deletion-shortcut-filters-5750.patch Paul Marillonnet, 12 Oct 2021 10:52 AM
0001-rbac-add-created-deleted-fields-on-RoleParenting-mod.patch (3.23 KB) 0001-rbac-add-created-deleted-fields-on-RoleParenting-mod.patch Paul Marillonnet, 12 Oct 2021 10:52 AM
0004-rbac-perform-soft-deletion-of-roleparenting-pairings.patch (1.9 KB) 0004-rbac-perform-soft-deletion-of-roleparenting-pairings.patch Paul Marillonnet, 12 Oct 2021 10:52 AM
0005-rbac-update_or_create-roleparenting-for-update_trans.patch (1.75 KB) 0005-rbac-update_or_create-roleparenting-for-update_trans.patch Paul Marillonnet, 12 Oct 2021 10:52 AM
0006-apply-soft-deletion-logic-when-retrieving-roleparent.patch (13.1 KB) 0006-apply-soft-deletion-logic-when-retrieving-roleparent.patch Paul Marillonnet, 12 Oct 2021 10:52 AM
0007-rbac-update_or_create-roleparenting-for-update_trans.patch (1.87 KB) 0007-rbac-update_or_create-roleparenting-for-update_trans.patch Paul Marillonnet, 15 Oct 2021 10:42 AM
0008-apply-soft-deletion-logic-when-retrieving-roleparent.patch (11.5 KB) 0008-apply-soft-deletion-logic-when-retrieving-roleparent.patch Paul Marillonnet, 15 Oct 2021 10:42 AM
0006-rbac-perform-soft-deletion-of-roleparenting-pairings.patch (1.71 KB) 0006-rbac-perform-soft-deletion-of-roleparenting-pairings.patch Paul Marillonnet, 15 Oct 2021 10:42 AM
0005-rbac-soft_delete-requires-transitive-closure-update-.patch (1.2 KB) 0005-rbac-soft_delete-requires-transitive-closure-update-.patch Paul Marillonnet, 15 Oct 2021 10:42 AM
0004-rbac-define-post_soft_delete-signal-and-its-handler-.patch (1.72 KB) 0004-rbac-define-post_soft_delete-signal-and-its-handler-.patch Paul Marillonnet, 15 Oct 2021 10:42 AM
0003-rbac-define-custom-manager-for-active-objects-57500.patch (1.44 KB) 0003-rbac-define-custom-manager-for-active-objects-57500.patch Paul Marillonnet, 15 Oct 2021 10:42 AM
0002-rbac-add-soft-created-deleted-manager-methods-57500.patch (1.78 KB) 0002-rbac-add-soft-created-deleted-manager-methods-57500.patch Paul Marillonnet, 15 Oct 2021 10:42 AM
0001-rbac-add-created-deleted-fields-on-RoleParenting-mod.patch (3.23 KB) 0001-rbac-add-created-deleted-fields-on-RoleParenting-mod.patch Paul Marillonnet, 15 Oct 2021 10:42 AM

History

#1

Updated by Paul Marillonnet 24 days ago

  • Assignee set to Paul Marillonnet
#2

Updated by Paul Marillonnet 19 days ago

  • Status changed from Nouveau to En cours

Hmm, j’ai commencé à pousser un truc dans la branche. Le build laisse à penser que django teste la possibilité d’évaluer certaines des expressions F() du patch avant l’exécution de la migration. Je regarde.

#4

Updated by Benjamin Dauvergne 13 days ago

L'implémentation de soft_delete() me semble ok, mais soft_create() je ne pense pas qu'on voudrait l'exposer comme ça, mais j'ai pas encore tout lu, je continue.


Sur un "soft create" il faut remettre "deleted" à NULL, et donc pas besoin de filtre "deleted__lte=F('created')" le seul filtre à avoir c'est deleted__isnull=True quand on cherche les parents/enfants d'un rôle.

Le plus simple c'est d'ajouter un

from model_utils.manager import QueryManager
...
class RoleParenting:
...

actives = QueryManager(deleted__isnull=True)


Plutôt que d'ajouter des soft_create/soft_delete partout je mettrais toute la mécanique create/delete dans des méthodes de RoleParentingManager (qui sera instantié sur RoleParenting.objects) :

class RoleParentingManager:
...
    def soft_create(self, *, parent, child):
         with atomic(savepoint=False):
             rp = self.get_or_crate(parent=parent, child=child)
             if rp.deleted:
                 rp.created = now()
                 rp.deleted = None
                 rp.save(update_fields=['created', 'deleted'])
...
    def soft_delete(self, *, parent, child):
        self.filter(parent=parent, child=child, deleted__isnull=True).update(deleted=now())


Le contraire pour update_transitive_closure, je préfère que la mécanique soit visible ici, c'est déjà du code assez compliqué.


0006: voir si on peut faire en sorte que les managers parent/child_relation utilise le Manager "actives" et pas "objects", et aussi remplacer tous les RoleParenting.objects. par RoleParenting.actives. (on peut aussi poser QueryManager sur objects et définir un "all = Manager()", la doc https://docs.djangoproject.com/en/3.2/topics/db/managers/#default-managers )

#5

Updated by Benjamin Dauvergne 13 days ago

Visiblement avec ça on pourrait s'en sortir :

class RoleParentingManager(Manager):
    def get_queryset(self):
        return super().get_queryset().filter(deleted__isnull=True)
....
class RoleParenting:
    objects = RoleParentingManager()
    all = models.Manager()

    class Meta:
        base_manager_name = 'all'

#6

Updated by Paul Marillonnet 12 days ago

  • Status changed from Solution proposée to En cours

Yes c’est bien vu, j’ai commencé à pousser des modifications qui vont dans ce sens, je regarde comment intégrer ça de façon cohérente.

#7

Updated by Paul Marillonnet 11 days ago

Paul Marillonnet a écrit :

Yes c’est bien vu, j’ai commencé à pousser des modifications qui vont dans ce sens, je regarde comment intégrer ça de façon cohérente.

C’est intégré dans la branche. Il me reste quelques modifications pour gérer les signaux déclenchant le re-calcul de la fermeture transitive (les soft_create/delete sur le manager ne semble pas le déclencher).

#8

Updated by Benjamin Dauvergne 11 days ago

Paul Marillonnet a écrit :

C’est intégré dans la branche. Il me reste quelques modifications pour gérer les signaux déclenchant le re-calcul de la fermeture transitive (les soft_create/delete sur le manager ne semble pas le déclencher).

Je n'ai pas (re)regardé la branche mais ça parait normal, .update() ne déclenche pas de signaux.

#9

Updated by Paul Marillonnet 11 days ago

Benjamin Dauvergne a écrit :

Paul Marillonnet a écrit :

C’est intégré dans la branche. Il me reste quelques modifications pour gérer les signaux déclenchant le re-calcul de la fermeture transitive (les soft_create/delete sur le manager ne semble pas le déclencher).

Je n'ai pas (re)regardé la branche mais ça parait normal, .update() ne déclenche pas de signaux.

Voilà, ça m’a l’air mieux comme ça. Patches avec définition d’un signal et son handler (0004) et son usage après le .update() (0005).

Also available in: Atom PDF