user: allow customization of User.get_full_name() through templates (#72945) #2
Loading…
Reference in New Issue
No description provided.
Delete Branch "wip/72945-custom-rendered-name-via-template"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Via une app à installer manuellement sur les briques concernées qui s'occupe de monkey-patcher le modèle user. Ça permet d'avoir un rendu cohérent partout.
Pour tester, par exemple sur combo :
'hobo.user_name.apps.UserNameConfig',
danscombo/settings.py:INSTALLED_APPS
echo '{{ user.first_name }} Bar' > templates/variants/univ-avignon/includes/user-info-user-name.html
admin Bar
ou approchantcc @fpeters parce qu'on a élaboré l'approche ensemble.
Pour tester. Une fois qu'on est ok on peut installer cette partie via debian/debian_config_common.py, dans la section qui installe déjà le provisionning :
Ou juste dans une branche
PROJECT_NAME != 'wcs'
, en ajoutant au code la détection du fait qu'il est appelé depuis authentic, ou en modifiant le code pour que l'attribut soit nommé "attributes" plutôt que "cached_extra_attributes" ce qui de loin me semble suffisant pour assurer le comportement à l'identique. (et côté w.c.s. l'attribut "attributes" n'existe actuellement pas, il pourra être ajouté pour compatibilité).Second commit pour réduire le souci de build qui crashe (une sombre histoire de settings preloadés et pas altérables dans les tests sans faire crasher le reste)
@ -289,2 +289,4 @@
'hobo.provisionning.middleware.ProvisionningMiddleware',
)
if PROJECT_NAME != 'wcs':
INSTALLED_APPS += ('hobo.user_name.apps.UserNameConfig',)
@fpeters ça te parait okay ? J'ai modifié la logique dans le
utils.py
pour avoir un comportement distinct côté authentic.@ -0,0 +31,4 @@
def cached_extra_attributes(user):
if 'authentic2' in settings.INSTALLED_APPS:
# we're in authentic, attributes are looked up differently
return {k: v.to_python() for k, v in user.attributes.values.items()}
Pas sûre de la meilleure façon de faire ça. Idéalement j'aimerai avoir la même payload que ce qui est envoyé lors du
GET /__provision__
sur chaque service mais je n'ai pas trouvé où elle est générée.Oui en effet il me semble que ça ne va pas fonctionner pour les attributs multiples dans authentic. Je regarde.
Il y a encore un souci de récursion, comme en témoignent les builds successifs dans les deux branches concernées.
À mon avis, il y a un endroit dans la gestion des erreurs où User.str ou User.repr est appelé, lequel appelle à son tour la nouvelle méthode monkeypatchée User.get_full_name, laquelle lève un TemplateNotFoundError, etc.
J’ai un peu creusé l’affaire mais sans trop d’inspiration, je pense que le monkeypatching se passe mal et que User.repr et User.str sont mal monkeypatchées et font appel à cette nouvelle méthode par gabarit. Je vais reprendre à tête reposée.
––––
En remarque plus générale, je note que j’aime moyen les substitutions automagiques sous le tapis telle que celle de ce patch, j’aurais préféré que le patch ajoute une méthode différente de get_full_name, par exemple get_templated_full_name, et qu’on adapte explicitement les gabarits partout où c’est nécessaire, par exemple en y ajoutant un truc du genre {% firstof user.get_templated_full_name user.get_full_name %}. Là déjà à l’écriture du patch on se tape un sale bug lorsque le gabarit du nom complet est absent dans les tests, c’est de mauvais augure pour la suite.
Pour l’instant j’ai retravaillé le patch pour y inclure la gestion des attributs multiples. Une petite modification dans les tests aussi pour y inclure directement le dossier contenant le gabarit à tester (via le setting TEMPLATES), plutôt que de déclarer et installer une app qui ne contient que ce dossier (et de la déclaration via INSTALLED_APPS).
Il reste encore ce souci de récursion à régler à l’appel des tests authentic lorsque que le gabarit personnalisé est inexistant, je regarde plus tard aujourd’hui.
5c828a7524
to0c14103e4b
Voilà, une nouvelle version qui corrige le souci de récursion dans les tests évoqué plus haut, et qui ajoute et teste le support des attributs multiples.
Je remonte à un truc que j'écrivais :
Il me semble que le code serait bien plus simple ainsi, zéro besoin de copier tout ce code authentic; pour authentic on garde le .attributes. natif qui fonctionne, pour les autres on ajoute un
.attributes = functools.cached_property(cached_extra_attributes)
.0c14103e4b
to90f441d72d
90f441d72d
to729c174bf9
Oui ok j’avais loupé cette remarque en reprenant le ticket en cours de route. C’est corrigé dans la branche et c’est bien mieux ainsi, en effet.
C'était dans le plan initial,
Mais je pense maintenant qu'il ne faut pas faire ainsi, que le gabarit doit plutôt être pris d'une variable hobo (qui se retrouvent dans tous les modules via settings.TEMPLATE_VARS).
L'idée c'est que ça n'est pas en soit lié à une intégration graphique et que ça permettra dans un second temps d'ajouter la configuration de cette partie via l'UI (via l'écran profil d'hobo qui permet déjà de définir les attributs).
Ici je ne demande pas cette UI, mais bien que le gabarit soit récupéré depuis
settings.TEMPLATE_VARS['user_full_name_template']
, elle y arriverait pour le moment via l'écran générique "variables".729c174bf9
toab28c31b2d
@ -0,0 +23,4 @@
django.template.exceptions.TemplateDoesNotExist,
django.template.exceptions.TemplateSyntaxError,
) as e:
logger.exception('hobo.user_name: cannot render user "%s" name', user)
Ça ne me semble pas adéquat ça va spammer d'exceptions partout alors que ne pas avoir de gabarit personnalisé reste le cas normal.
C'est peut-être plus simple de ne pas découper autant, ramener le code de get_full_name_from_template dans cette fonction, et si
if 'user_full_name_template' not in template_vars
alorsreturn user.original_get_full_name()
, et laisser alors la gestion des exceptions pour les vrais cas exceptionnels.(désolé de ne pas avoir pu tout noter d'un coup).
Oui complètement, j’avais loupé ça, merci.
Ok, c’est modifié dans la branche.
J’ai laissé le ´except UserExtraAttributes.DoesNotExist´ dans le code bien qu’il ne me semble pas nécessaire, mais je me dis que peut-être j’ai loupé un cas limite qu’Agate aurait identifié pour certaines briques (?)
(absolument aucun souci)
Oui ok, pourquoi pas, c’est sans doute mieux comme ça sans avoir à toucher au thème, en effet.
C’est à jour dans la branche, on cherche maintenant à retrouver le contenu d’un gabarit inline dans ´settings.TEMPLATE_VARS['user_full_name_template']´ au lieu du contenu d’un fichier.
ab28c31b2d
to1282779c1d