Arborescence de groupes

tbastian commented

2019-12-13 18:03:26 +01:00

(Migrated from git.eleves.ens.fr)

Il faudrait pouvoir avoir des groupes arborescents : par exemple, Nuit devrait être un méta-groupe contenant COF, Nuit 19, Nuit 18, … COF étant lui-même un groupe contenant COF 19 et COF 18 par exemple. Cf l'issue sur django-wiki ici (TL;DR : ils ne veulent pas implémenter ça upstream).

On peut changer le modèle utilisé pour la gestion des groupes avec le setting WIKI_GROUP_MODEL, mais c'est un peu risqué. On peut aussi changer la fonction qui détermine si on peut lire/écrire un truc, mais ça demande de refaire toute l'UI. Après un regard rapide dans le code, ça n'a pas l'air trop risqué en fait, il faudrait expérimenter.

On pourrait en profiter pour implémenter #2.

Il faudrait pouvoir avoir des groupes arborescents : par exemple, `Nuit` devrait être un méta-groupe contenant `COF`, `Nuit 19`, `Nuit 18`, … `COF` étant lui-même un groupe contenant `COF 19` et `COF 18` par exemple. Cf l'issue sur `django-wiki` [ici](https://github.com/django-wiki/django-wiki/issues/999) (TL;DR : ils ne veulent pas implémenter ça upstream). On peut changer le modèle utilisé pour la gestion des groupes avec le setting `WIKI_GROUP_MODEL`, mais c'est [un peu risqué](https://github.com/django-wiki/django-wiki/issues/801). On peut aussi changer la fonction qui détermine si on peut lire/écrire un truc, mais ça demande de refaire toute l'UI. Après un regard rapide dans le code, ça n'a pas l'air trop risqué en fait, il faudrait expérimenter. On pourrait en profiter pour implémenter #2.

tbastian commented

2019-12-15 15:58:46 +01:00

(Migrated from git.eleves.ens.fr)

Résumé de discussions @mpepin et @tbastian sur Merle [KDE/Général] :

Donc en gros, les deux options c'est :

on fait un modèle arborescent de groupes de notre côté, on le piggy-back sur les groupes Django avec du scotch, on a rien de plus à faire et on est contents, et éventuellement à part on fait une interface de gestion des groupes qui se plug très bien là-dessus (il suffit qu'elle touche à "nos" groupes)

on fait le même modèle, on réimplem les can_* et on refait nécessairement l'interface + ça a des chances de casser là où les devs ont supposé qu'on utilisait les groupes

(typiquement j'ai peur que la récursion automatique des perms casse : si on définit nos can_*, we're on our own et on doit s'assurer qu'une page nouvellement créée blah/truc hérite des perms de blah)

Du coup [1] est a priori adopté.

Résumé de discussions @mpepin et @tbastian sur Merle [KDE/Général] : > Donc en gros, les deux options c'est : > > 1) on fait un modèle arborescent de groupes de notre côté, on le piggy-back sur les groupes Django avec du scotch, on a rien de plus à faire et on est contents, et éventuellement à part on fait une interface de gestion des groupes qui se plug très bien là-dessus (il suffit qu'elle touche à "nos" groupes) > > 2) on fait le même modèle, on réimplem les can_* et on refait nécessairement l'interface + ça a des chances de casser là où les devs ont supposé qu'on utilisait les groupes > > (typiquement j'ai peur que la récursion automatique des perms casse : si on définit nos can_*, we're on our own et on doit s'assurer qu'une page nouvellement créée blah/truc hérite des perms de blah) Du coup [1] est a priori adopté.

mpepin commented

2019-12-15 16:01:24 +01:00

(Migrated from git.eleves.ens.fr)

Et du coup, on est d'accord que ça donne un truc comme ça ?

class MyGroup(Model):
    django_group = OneToOne(Group)
    parent = ForeignKey("MyGroup", blank=True, null=True)
    ...

Edit j'ai rajoué le blank=True, null=True après

Et du coup, on est d'accord que ça donne un truc comme ça ? ```python class MyGroup(Model): django_group = OneToOne(Group) parent = ForeignKey("MyGroup", blank=True, null=True) ... ``` **Edit** j'ai rajoué le `blank=True, null=True` après

tbastian commented

2019-12-15 16:04:15 +01:00

(Migrated from git.eleves.ens.fr)

À la réflexion je suis même pas si sûr que c'est une arborescence et pas un DAG.

Typiquement on a

COF18 ----- COF ----- Nuit
        /         \
COF19 -/           \- K-Fêt (maybe)

Du coup c'est plutôt un truc avec un ManyToMany qui pointe les nœuds fils ?

En fait je pense que la bonne manière de se représenter ça c'est de définir dans un groupe quels membres lui sont propres, et quels groupes ce groupe inclut. On pourrait aussi se demander si pour que ça soit plus clair à lire, un groupe ne peut contenir que l'un des deux (soit un groupe est un métagroupe qui inclut des groupes, soit il contient des gens directement).

À la réflexion je suis même pas si sûr que c'est une arborescence et pas un DAG. Typiquement on a ``` COF18 ----- COF ----- Nuit / \ COF19 -/ \- K-Fêt (maybe) ``` Du coup c'est plutôt un truc avec un `ManyToMany` qui pointe les nœuds fils ? En fait je pense que la bonne manière de se représenter ça c'est de définir dans un groupe quels membres lui sont propres, et quels groupes ce groupe inclut. On pourrait aussi se demander si pour que ça soit plus clair à lire, un groupe ne peut contenir que l'un des deux (soit un groupe est un métagroupe qui inclut des groupes, soit il contient des gens directement).

mpepin commented

2019-12-15 16:31:11 +01:00

(Migrated from git.eleves.ens.fr)

Oui, en fait je crois qu'on ne va pas échapper au manytomany…

Par contre est-ce que ça va vraiment être plus clair à lire de séparer les deux cas ?
Sachant que dans ton modèle il y aura forcément un M2M vers les groupes django et un M2M vers les metagroups. Malheureusement je ne crois pas qu'on puisse faire un "ou" propre dans l'ORM django.

Oui, en fait je crois qu'on ne va pas échapper au manytomany… Par contre est-ce que ça va vraiment être plus clair à lire de séparer les deux cas ? Sachant que dans ton modèle il y aura forcément un M2M vers les groupes django et un M2M vers les metagroups. Malheureusement je ne crois pas qu'on puisse faire un "ou" propre dans l'ORM django.

tbastian commented

2019-12-15 16:32:47 +01:00

(Migrated from git.eleves.ens.fr)

Je parle pas de propre dans le code, mais de propre dans l'architecture des groupes : que ça soit clair pour un humain qui regarde le graphe des groupes que si COF18 et COF19 contiennent des gens, alors COF ne contient pas de personne random rajoutée au niveau du métagroupe COF directement. Dans le code je sais pas si on peut faire ça joliment, peut-être qu'avec de la validation ça se fait ?

mpepin commented

2019-12-15 16:34:50 +01:00

(Migrated from git.eleves.ens.fr)

Dans le code on peut mettre un champ group_type qui vaut META ou REAL et on ne présente pas le même formulaire à la personne qui manage les groupes selon ce champ.

La validation ça marche aussi.

Dans le code on peut mettre un champ `group_type` qui vaut `META` ou `REAL` et on ne présente pas le même formulaire à la personne qui manage les groupes selon ce champ. La validation ça marche aussi.

mpepin commented

2019-12-15 16:36:02 +01:00

(Migrated from git.eleves.ens.fr)

Ou encore on peut mélanger les deux, parce que pourquoi pas, mais on met un champ booléen sur les groupes qui dit si on a le droit de mettre des utilisateurs dans le groupe ou juste des sous-groupes

tbastian commented

2019-12-15 16:36:42 +01:00

(Migrated from git.eleves.ens.fr)

Ou alors on fait juste confiance aux gens pour ne pas mélanger les deux, je ne sais pas si c'est vraiment au code d'enforce ça.

mpepin commented

2019-12-15 16:40:01 +01:00

(Migrated from git.eleves.ens.fr)

Ça peut être une feature de mélanger les deux au pire.

En tout cas la v1 peut s'en foutre et on verra selon comment c'est utilisé s'il faut faire des modifs dans un deuxième temps

Ça peut être une feature de mélanger les deux au pire. En tout cas la v1 peut s'en foutre et on verra selon comment c'est utilisé s'il faut faire des modifs dans un deuxième temps

mpepin commented

2019-12-15 16:45:52 +01:00

(Migrated from git.eleves.ens.fr)

Pour moi la v1 c'est :

un modèle simple avec deux M2M
une fonction pour répercuter sur les groupes django

Et assez vite ensuite :

détection de cycles
décider si on modifie les groupes django dans un post_save ou si veut que ça se fasse par un appel explicite à la bonne fonction

Pour moi la v1 c'est : - un modèle simple avec deux M2M - une fonction pour répercuter sur les groupes django Et assez vite ensuite : - détection de cycles - décider si on modifie les groupes django dans un `post_save` ou si veut que ça se fasse par un appel explicite à la bonne fonction

tbastian commented

2019-12-15 16:47:47 +01:00

(Migrated from git.eleves.ens.fr)

C'est quoi tes deux M2M ? C'est pas un M2M pour l'inclusion de groupes + un O2O pour mapper un groupe arborescent sur un groupe Django ?

Sinon, pour moi c'est assez clair qu'on veut mettre ça dans post_save, non ? Par contre, le truc à voir c'est si on veut mettre un post_save dans l'autre sens pour vérifier que personne n'est en train de toucher aux groupes à la main.

Et la détection de cycles c'est pas bien compliqué je pense, c'est dans la validation du groupe que ça doit se faire je pense.

C'est quoi tes deux M2M ? C'est pas un M2M pour l'inclusion de groupes + un O2O pour mapper un groupe arborescent sur un groupe Django ? Sinon, pour moi c'est assez clair qu'on veut mettre ça dans `post_save`, non ? Par contre, le truc à voir c'est si on veut mettre un `post_save` dans l'autre sens pour vérifier que personne n'est en train de toucher aux groupes à la main. Et la détection de cycles c'est pas bien compliqué je pense, c'est dans la validation du groupe que ça doit se faire je pense.

mpepin commented

2019-12-15 16:57:05 +01:00

(Migrated from git.eleves.ens.fr)

Ah on avait pas le même truc en tête :

je voyais un M2M subgroups vers MyGroup et un autre users vers User + un O2O vers un groupe Django
mais effectivement on peut virer le deuxième en disant que les utilisateurs proviennent du groupe Django, c'est beaucoup mieux

Oui post_save c'est bien, je divague. Dans l'autre sens c'est pas un post_save puisque post_save c'est trop tard. Mais je ne suis même pas sûr qu'on ait besoin de mettre de sécurité vu qu'on va probablement fermer l'accès à l'admin et permettre de toucher aux groupes seulement via notre interface.

C'est pas compliqué mais il y a quand même un tri topologique à écrire. Et oui, dans la validation.

Ah on avait pas le même truc en tête : - je voyais un M2M `subgroups` vers `MyGroup` et un autre `users` vers `User` + un O2O vers un groupe Django - mais effectivement on peut virer le deuxième en disant que les utilisateurs proviennent du groupe Django, c'est beaucoup mieux Oui `post_save` c'est bien, je divague. Dans l'autre sens c'est pas un `post_save` puisque `post_save` c'est trop tard. Mais je ne suis même pas sûr qu'on ait besoin de mettre de sécurité vu qu'on va probablement fermer l'accès à l'admin et permettre de toucher aux groupes seulement via notre interface. C'est pas compliqué mais il y a quand même un tri topologique à écrire. Et oui, dans la validation.

tbastian commented

2019-12-15 16:59:02 +01:00

(Migrated from git.eleves.ens.fr)

Ah, je suis pas certain que les users venant du groupe Django ça soit mieux en vrai, j'avais juste oublié le M2M vers les users. Je pense même que c'est moins bien parce que tant qu'à avoir du scotch on veut pas que ça soit dans les deux sens, si on peut avoir au moins une structure self contained propre c'est mieux je pense.

mpepin commented

2019-12-15 17:01:22 +01:00

(Migrated from git.eleves.ens.fr)

On duplique plus de trucs mais c'est probablement pas grave. Dans ce cas va pour deux 2M2 MyGroup + User et un O2O vers Group

On duplique plus de trucs mais c'est probablement pas grave. Dans ce cas va pour deux 2M2 `MyGroup` + `User` et un O2O vers `Group`

tbastian commented

2019-12-16 01:05:34 +01:00

(Migrated from git.eleves.ens.fr)

mentioned in merge request !8

tbastian commented

2019-12-16 01:09:26 +01:00

(Migrated from git.eleves.ens.fr)

C'est un peu plus compliqué que prévu pour le post_save en fait, pour deux raisons :

on doit tenir compte du fait que post_save ne marche que pour le modèle et pas pour le m2m, donc on doit gérer trois signaux en fait (cf le code dans la MR),
on doit penser à commit récursivement tous nos métagroupes. Il faudrait peut-être faire ça intelligemment histoire de ne pas refaire un nombre exponentiel de fois le même taf' quand on a une structure de graphe en diamant (ou alors on ignore le problème parce que personne ne fera un graphe designé pour nous faire exploser).

C'est un peu plus compliqué que prévu pour le `post_save` en fait, pour deux raisons : * on doit tenir compte du fait que `post_save` ne marche que pour le modèle et pas pour le m2m, donc on doit gérer trois signaux en fait (cf le code dans la MR), * on doit penser à commit récursivement tous nos métagroupes. Il faudrait peut-être faire ça intelligemment histoire de ne pas refaire un nombre exponentiel de fois le même taf' quand on a une structure de graphe en diamant (ou alors on ignore le problème parce que personne ne fera un graphe designé pour nous faire exploser).

tbastian commented

2019-12-20 17:07:49 +01:00

(Migrated from git.eleves.ens.fr)

closed via merge request !8