Development #10530
combo.utils.requests est un peu magique sur la détection de l'utilisateur
0%
Description
Je dirais même plus, un peu trop magique.
Je préférerai avoir des appels comme ceci :
response = requests.get(self.api_url, remote_service=wcs_site, user=user,
plutôt que l'envoi de « request=context.get('request') » qui va aller chercher un user on ne sait trop comment.
L'idée derrière c'est qu'une cellule compatible avec usersearch pourra faire un clair « user = context.get('selected_user') or user »
Fichiers
Révisions associées
Historique
Mis à jour par Thomas Noël il y a environ 8 ans
Mis à jour par Thomas Noël il y a environ 8 ans
- Statut changé de Nouveau à En cours
- Patch proposed changé de Non à Oui
Mis à jour par Frédéric Péters il y a environ 8 ans
J'aime bien l'idée de ne pas avoir à ajouter
user = context.get('selected_user') or getattr(context.get('request'), 'user', None)
à tous les appels; peut-être ajouter un get_concerned_user() à CellBase, qui ferait ça ? Et les appels à requests auraient la plupart un user=self.get_concerned_user(), ça m'irait.
À côté de ça, du coup je trouve trop magique cette sélection du nameid ou de l'email, j'ajouterais un paramètre federation_key='auto' (autres valeurs possibles 'email' et 'nameid').
Mis à jour par Thomas Noël il y a environ 8 ans
- Fichier 0001-utils-explicit-user-in-requests-10530.patch 0001-utils-explicit-user-in-requests-10530.patch ajouté
Nouvelle proposition ci-jointe
Mis à jour par Frédéric Péters il y a environ 8 ans
Il me semble que s'est perdue la situation de mettre NameID= et email= vides, quand la requête doit être faite au nom d'un usager et qu'il n'y en a pas (ce qui fera la différence entre une requête "les démarches accessibles anonymement" et "toutes les démarches"). C'est de là que venait la différence entre le AUTO_USER en valeur par défaut et l'explicite user=None dans get_wcs_json.
Je pense là qu'on pourrait avoir query_params['NameID'] = None et query_params['email'] = None positionnés au début, écrasés/retirés quand on est sur le isinstance(), retirés quand un dictionnaire est passé (peut-être en mettant le 'orig' derrière tout ça, pour faire user.copy() en situation de dictionnaire).
Mis à jour par Thomas Noël il y a environ 8 ans
Frédéric Péters a écrit :
Il me semble que s'est perdue la situation de mettre NameID= et email= vides
Quid de faire un appel avec « user={'NameID': ''} » ?
Mis à jour par Frédéric Péters il y a environ 8 ans
Quid de faire un appel avec « user={'NameID': ''} » ?
Ça veut dire dans le code de chaque cellule faire "if self.get_concerned_user(): prendre celui-là, else: prendre {'NameId': ''}); je préfère plutôt demander un paramètre supplémentaire (avant, user=None) pour les appels devant explicitement ne pas mentionner d'utilisateur.
Mis à jour par Thomas Noël il y a environ 8 ans
- Fichier 0001-utils-explicit-user-in-requests-10530.patch 0001-utils-explicit-user-in-requests-10530.patch ajouté
Je n'arrive pas à être satisfait, mais voici.
Mis à jour par Frédéric Péters il y a environ 8 ans
le isinstance(user, get_user_model()) casse le duck typing dont on profite dans les tests :/
Inverser alors ça et d'abord tester "if isinstance(user, dict)" puis simplement "if user" ?
elif user:
# user must then be a dictionary
> query_params = user.copy()
E AttributeError: 'MockUser' object has no attribute 'copy'
(ou adapter les tests)
Mis à jour par Thomas Noël il y a environ 8 ans
- Fichier 0001-utils-explicit-user-in-requests-10530.patch 0001-utils-explicit-user-in-requests-10530.patch ajouté
L'inversion du test est très bien.
Mis à jour par Thomas Noël il y a environ 8 ans
- Statut changé de En cours à Résolu (à déployer)
commit 462aada3f860272db6fbbc5e3f3f8ddb9daa10a5
Author: Thomas NOEL <tnoel@entrouvert.com>
Date: Fri Apr 1 18:50:41 2016 +0200
utils: explicit user in requests (#10530)
Mis à jour par Frédéric Péters il y a presque 8 ans
- Statut changé de Résolu (à déployer) à Fermé
utils: explicit user in requests (#10530)