Produits Entr'ouvert » Authentic 2

Would authentic2.utils.login() be a good placement for this callback ? (reposted in #14070)

Emmanuel Cazenave a écrit :

Je peux pas acker si ya pas un test.

Ok.

Deux import inutiles dans 0002-manager-allow-overriding-resource-class-in-export-vi.patch.

Ok.

Et aussi, vieux débat, mais pas pour ma part pas fan de l'utilisation qui est faite de l'héritage : avec méthode 'finale' get_dataset dans la classe mère, dont le comportement est modifié dans une classe fille parce qu'on y redéfinit get_resource.
Je trouverais ça plus clair si c'était get_dataset qui était redéfinie.

J'ai l'impression de suivre le modèle générale des vues générique que ce soit dans Django(get_form_class, get_form_kwargs, get_form) ou dans DRF (pareil avec get_serializer, etc...). Dès qu'un objet un peu utilisé partout dans la vue est construit on déplace le constructeur dans une méthode à part pour pouvoir le customiser. Là tout de suite je ne vois pas le besoin de changer quelque chose.

La surcharge actuelle de get_dataset() sert uniquement à contourner un problème avec django-import-export qui annule les effets du prefetching quand on passe un queryset (voir https://github.com/django-import-export/django-import-export/issues/774).

Je pense que je vais remonter ça au niveau de classe mère parce que ça n'a rien de spécifique.

Toujours les deux imports inutiles qui trainent.

J'essayais de comprendre à coup de pdb ce qui allait se passer sur qs = super(UsersExportView, self).get_queryset() parce qu'avais l'impression
qu'aucune classe parente ne définit cette méthode, mais j'ai pas pu arriver jusque là, le test plante.

NameError: global name 'superuser' is not defined

Ma remarque sur l'héritage, de façon plus formelle c'est ça : https://codeburst.io/understanding-solid-principles-open-closed-principle-e2b588b6491f
Mais d'une part tout le monde est pas d'accord, d'autre part sur ton code c'était suffisamment petit pour que ce soit pas un gros problème.
Mais quand ça devient plus gros, genre un héritage à 3 niveaux et des redéfinition de certaines méthodes dans les classes filles qui seront appelées par les classe mères, ça peut vite devenir tout à fait incompréhensible.

Tout ça pour dire fais comme tu veux.

Corrigé les deux soucis, je vais pousser quand mes tests ont fini de tourner.

Pour le reste, j'aime bien ces idées (préférer la composition à l'héritage, has-a plutôt que is-a), mais Django n'est pas du tout construit en suivant ce principe, c'est de l'héritage partout avec des bouts de composition (tous les systèmes avec des backends), je préfère la cohérence avec notre framework de base plutôt que d'avoir à empiler des styles de code différents (je pense que ce serait encore pire). Ça veut dire qu'un regarde extérieur habitué à du code Django (ou regardant des exemples dans les docs) ne sera pas trop perdu.