toulouse-maelis: get users linked to a family (#77295) #239

Merged
nroche merged 1 commits from wip/77295-parsifal-get-linked-users into main 2023-11-10 10:38:06 +01:00
Owner

Endpoint qui liste les NameID appaîrés à une famille,
complété avec les informations de l'usager obtenus via authentic.

Endpoint qui liste les NameID appaîrés à une famille, complété avec les informations de l'usager obtenus via authentic.
fpeters force-pushed wip/77295-parsifal-get-linked-users from d25159357a to 1b754bc3ba 2023-05-09 10:42:59 +02:00 Compare
Owner

à rebaser.

à rebaser.
fpeters requested changes 2023-06-29 18:39:11 +02:00
Dismissed
fpeters left a comment
Owner

à rebaser.

J'explicite le changement dans redmine via "demander des changements".

(ça ne m'engage pas à ensuite relire)

> à rebaser. J'explicite le changement dans redmine via "demander des changements". (ça ne m'engage pas à ensuite relire)
nroche force-pushed wip/77295-parsifal-get-linked-users from 1b754bc3ba to 8ec1489154 2023-06-30 15:53:50 +02:00 Compare
Author
Owner

à rebaser.

Il y avait un effet de bord sur les services définis par la fixture wcs_service.
J'ai rajouté 2 commits pour remettre ça d’équerre.

> à rebaser. Il y avait un effet de bord sur les services définis par la fixture wcs_service. J'ai rajouté 2 commits pour remettre ça d’équerre.
nroche dismissed fpeters’s review 2023-06-30 16:18:08 +02:00
Reason:

(ça ne m'engage pas à ensuite relire)

Author
Owner

nroche a rejeté la revue de fpeters maintenant

J'ai mis un commentaire mais il n'a pas été repris, c'était :

(ça ne m'engage pas à ensuite relire)

> nroche a rejeté la revue de fpeters maintenant J'ai mis un commentaire mais il n'a pas été repris, c'était : > (ça ne m'engage pas à ensuite relire)
nroche force-pushed wip/77295-parsifal-get-linked-users from 8ec1489154 to 019eb8572d 2023-07-10 10:38:46 +02:00 Compare
tnoel requested changes 2023-07-10 12:21:35 +02:00
tnoel left a comment
Owner

Je n'ai pas compris le texte du commit «  toulouse-maelis: [tests] explicitely remove wcs service (#77295) » vu ce qu'il contient...

Je n'ai pas compris le texte du commit «  toulouse-maelis: [tests] explicitely remove wcs service (#77295) » vu ce qu'il contient...
@ -1140,0 +1155,4 @@
)
def get_link_list(self, request, NameID=None, family_id=None, text_template=None):
if not text_template:
text_template = '{{ first_name }} {{ last_name }}'
Owner

Il faut mettre un « text_template='{{ first_name }} {{ last_name }}' » dans la définition de la fonction afin qu'on voit que c'est la valeur par défaut.

Mais en fait je pense qu'on pourrait ce passer de cette option qui sera compliquée à utiliser.

Et j'afficherais plus d'informations, quelque chose comme

prénom nom <email> (lié le jj/mm/aaaa ; compté créé le jj/mm/aaaa, dernière connexion le jj/mm/aaaa)

et trier la liste par ordre de date de création du Link. Ca aidera l'agent à savoir quoi faire.

Il faut mettre un « text_template='{{ first_name }} {{ last_name }}' » dans la définition de la fonction afin qu'on voit que c'est la valeur par défaut. Mais en fait je pense qu'on pourrait ce passer de cette option qui sera compliquée à utiliser. Et j'afficherais plus d'informations, quelque chose comme `prénom nom <email> (lié le jj/mm/aaaa ; compté créé le jj/mm/aaaa, dernière connexion le jj/mm/aaaa)` et trier la liste par ordre de date de création du Link. Ca aidera l'agent à savoir quoi faire.
Author
Owner

Il faut mettre un « text_template='...' » dans la définition de la fonction afin qu'on voit que c'est la valeur par défaut.

Oui sinon la valeur par défaut n'apparaît pas.
J'ai corrigé pour l'existant, merci.

Mais en fait je pense qu'on pourrait ce passer de cette option qui sera compliquée à utiliser.

Oui, j'ai créé un fichier template dédié et j'ai laissé tomber la possibilité de modifier le template.
J'y reviendrais plus tard s'il le faut.

Et j'afficherais plus d'informations, quelque chose comme
prénom nom (lié le jj/mm/aaaa ; compté créé le jj/mm/aaaa, dernière connexion le jj/mm/aaaa)

J'ai fait ça.

et trier la liste par ordre de date de création du Link. Ca aidera l'agent à savoir quoi faire.

Fait.

> Il faut mettre un « text_template='...' » dans la définition de la fonction afin qu'on voit que c'est la valeur par défaut. Oui sinon la valeur par défaut n'apparaît pas. J'ai corrigé pour l'existant, merci. > Mais en fait je pense qu'on pourrait ce passer de cette option qui sera compliquée à utiliser. Oui, j'ai créé un fichier template dédié et j'ai laissé tomber la possibilité de modifier le template. J'y reviendrais plus tard s'il le faut. > Et j'afficherais plus d'informations, quelque chose comme > prénom nom <email> (lié le jj/mm/aaaa ; compté créé le jj/mm/aaaa, dernière connexion le jj/mm/aaaa) J'ai fait ça. > et trier la liste par ordre de date de création du Link. Ca aidera l'agent à savoir quoi faire. Fait.
tnoel marked this conversation as resolved
nroche changed title from toulouse-maelis: get users linked to a family (#77295) to WIP: toulouse-maelis: get users linked to a family (#77295) 2023-07-10 18:01:59 +02:00
nroche force-pushed wip/77295-parsifal-get-linked-users from 019eb8572d to 431a2bf173 2023-07-10 18:02:19 +02:00 Compare
nroche force-pushed wip/77295-parsifal-get-linked-users from 431a2bf173 to 1905276970 2023-07-11 18:42:10 +02:00 Compare
nroche force-pushed wip/77295-parsifal-get-linked-users from 1905276970 to 72f559927e 2023-07-11 19:11:21 +02:00 Compare
Author
Owner

Il faut mettre un « text_template='...' » dans la définition de la fonction afin qu'on voit que c'est la valeur par défaut.

J'ai appliqué la correction partout.

> Il faut mettre un « text_template='...' » dans la définition de la fonction afin qu'on voit que c'est la valeur par défaut. J'ai appliqué la correction partout.
nroche changed title from WIP: toulouse-maelis: get users linked to a family (#77295) to toulouse-maelis: get users linked to a family (#77295) 2023-07-11 19:21:15 +02:00
nroche requested review from tnoel 2023-07-11 19:21:38 +02:00
fpeters reviewed 2023-07-11 20:58:10 +02:00
@ -725,4 +727,2 @@
result = self.get_family_raw(family_id)
if not text_template:
text_template = '{{ lastname }} {{ firstname }}'
Owner

Mais trois lignes plus bas cette méthode fait :

item['text'] = render_to_string(text_template, item).strip()

Clairement le paramètre text_template est obligatoire et ne peut pas être None. Il ne faudrait pas qu'il y ait =None dans la signature.

Et vraisemblbalement, un peu plus haut,

    def read_rl_list_raw(self, family_id, text_template='', income_year=None):

a un problème similaire : jamais on ne veut avoir un gabarit vide ici, il faudrait retirer le ='' de la signature.

Il y en a peut-être d'autres.

Mais trois lignes plus bas cette méthode fait : ``` item['text'] = render_to_string(text_template, item).strip() ``` Clairement le paramètre text_template est obligatoire et ne peut pas être None. Il ne faudrait pas qu'il y ait =None dans la signature. Et vraisemblbalement, un peu plus haut, ``` def read_rl_list_raw(self, family_id, text_template='', income_year=None): ``` a un problème similaire : jamais on ne veut avoir un gabarit vide ici, il faudrait retirer le ='' de la signature. Il y en a peut-être d'autres.
Author
Owner

Merci. Désolé d'avoir laissé passé ça.
Je n'en ai pas vu d'autres.

Merci. Désolé d'avoir laissé passé ça. Je n'en ai pas vu d'autres.
fpeters marked this conversation as resolved
@ -1140,0 +1155,4 @@
request,
NameID=None,
family_id=None,
):
Owner

Il y a un paramètre "text_template" décrit dans @endpoint(...), qui ne se retrouve pas ici.

Il y a un paramètre "text_template" décrit dans `@endpoint(...)`, qui ne se retrouve pas ici.
Author
Owner

Oui, je devais le retirer et je ne l'avais fait qu'à moitié.
Merci.

Oui, je devais le retirer et je ne l'avais fait qu'à moitié. Merci.
@ -1140,0 +1162,4 @@
for x in self.link_set.filter(family_id=family_id).values()
]
# call authentic to add context to data items
Owner

Ça ne devrait pas être nécessaire; on provisionne désormais tous les attributs des utilisateurs, user.extra_attributes.data, il faut plutôt utiliser ça.

>>> from mellon.models import UserSAMLIdentifier
>> >name_id = '2dcfab2fc0ce4b3ea83c2b83d33f92bc'
>>> UserSAMLIdentifier.objects.get(name_id=name_id).user.extra_attributes.data
{'id': 68577,
 'ou': 'default',
 'uuid': '2dcfab2fc0ce4b3ea83c2b83d33f92bc',
 'email': 'fpeters@entrouvert.org',
 'phone': None,
 'title': None,
 'mobile': None,
 ...

(cf combo/profile/utils.py)

Comme on est/sera sur une nouvelle on pourrait même passer par get_user_model() plutôt que UserSAMLIdentifier, et simplement récupérer via :
get_user_model(username=name_id).extra_attributes.data.

Ça ne devrait pas être nécessaire; on provisionne désormais tous les attributs des utilisateurs, `user.extra_attributes.data`, il faut plutôt utiliser ça. ``` >>> from mellon.models import UserSAMLIdentifier >> >name_id = '2dcfab2fc0ce4b3ea83c2b83d33f92bc' >>> UserSAMLIdentifier.objects.get(name_id=name_id).user.extra_attributes.data {'id': 68577, 'ou': 'default', 'uuid': '2dcfab2fc0ce4b3ea83c2b83d33f92bc', 'email': 'fpeters@entrouvert.org', 'phone': None, 'title': None, 'mobile': None, ... ``` (cf combo/profile/utils.py) Comme on est/sera sur une nouvelle on pourrait même passer par get_user_model() plutôt que UserSAMLIdentifier, et simplement récupérer via : `get_user_model(username=name_id).extra_attributes.data`.
Author
Owner

Oui, je suis complètement passé à côté, merci.

Oui, je suis complètement passé à côté, merci.
@ -1140,0 +1186,4 @@
except ValueError:
pass
for item in data:
item['text'] = render_template_to_string('toulouse_maelis/get_link_list.json', item['context'])
Owner

N'appelons pas .json des gabarits, c'est la grosse confusion; plutôt family_link_template.txt, peut-être.

N'appelons pas .json des gabarits, c'est la grosse confusion; plutôt family_link_template.txt, peut-être.
Author
Owner

Fait.

Fait.
nroche force-pushed wip/77295-parsifal-get-linked-users from 72f559927e to 77acf91aaa 2023-07-12 14:01:12 +02:00 Compare
nroche force-pushed wip/77295-parsifal-get-linked-users from 77acf91aaa to db0a15285b 2023-07-12 15:42:02 +02:00 Compare
nroche changed title from toulouse-maelis: get users linked to a family (#77295) to WIP: toulouse-maelis: get users linked to a family (#77295) 2023-07-12 15:42:29 +02:00
nroche changed title from WIP: toulouse-maelis: get users linked to a family (#77295) to toulouse-maelis: get users linked to a family (#77295) 2023-07-12 17:42:46 +02:00
tnoel refused to review 2023-07-13 12:16:35 +02:00
nroche force-pushed wip/77295-parsifal-get-linked-users from db0a15285b to 66de16de94 2023-07-17 15:04:27 +02:00 Compare
nroche force-pushed wip/77295-parsifal-get-linked-users from 66de16de94 to 1e2fa2f494 2023-08-25 12:04:46 +02:00 Compare
Author
Owner

Je n'ai pas compris le texte du commit «  toulouse-maelis: [tests] explicitely remove wcs service (#77295) » vu ce qu'il contient...

J'ai précisé le message : explicitely override wcs service from settings

> Je n'ai pas compris le texte du commit «  toulouse-maelis: [tests] explicitely remove wcs service (#77295) » vu ce qu'il contient... > > J'ai précisé le message : explicitely override wcs service from settings
nroche closed this pull request 2023-08-25 14:08:49 +02:00
nroche reopened this pull request 2023-08-25 14:10:16 +02:00
fpeters reviewed 2023-09-04 17:01:38 +02:00
@ -11295,2 +11375,3 @@
# basket was removed, send trigger to wcs
con.hourly()
with override_settings(KNOWN_SERVICES={}):
con.hourly()
Owner

pas de rapport ?

pas de rapport ?
Author
Owner

Il y avait un effet de bord sur les services définis par la fixture wcs_service.
J'ai placé ce code dans un commit séparé.
cf ici

Il y avait un effet de bord sur les services définis par la fixture wcs_service. J'ai placé ce code dans un commit séparé. cf [ici](https://git.entrouvert.org/entrouvert/passerelle/pulls/239/commits)
nroche force-pushed wip/77295-parsifal-get-linked-users from 1e2fa2f494 to 4b145cb01f 2023-09-23 21:41:07 +02:00 Compare
nroche force-pushed wip/77295-parsifal-get-linked-users from 4b145cb01f to a51d29393d 2023-11-02 17:34:03 +01:00 Compare
Author
Owner

Je reprend ce ticket.

  • Je reviens à la première version du patch : les utilisateurs ne sont plus provisionnés que vers combo, wcs, chrono et lingo (#82004).
  • J'y ai rapporté le template qui ajoute les dates de création du compte, du lien et de dernière connexion.
  • J'ai retiré les patchs qui corrigeaient les fixtures pour qu'on puisse les utiliser en même temps : on n'a pas d'appel à wcs et à authentic dans un même test.
  • J'ai déporté dans #83054 l'affichage des valeurs par défaut des gabarits utilisés dans les endpoints.

exemple de ce qu'on listera via w.c.s. :

$ curl 'https://parsifal-passerelle.dev.publik.love/toulouse-maelis/test/get-link-list?NameID=83f6e19feb2043d2aafb041aea445b2c' | jq ".data[].text"
"functest (lié le 02/11/2023)"
"admin12 administre <nroche@yopmail.org> (lié le 26/01/2023 ; compte créé le 06/04/2020, dernière connexion le 02/11/2023)"
Je reprend ce ticket. * Je reviens à la première version du patch : les utilisateurs ne sont plus provisionnés que vers combo, wcs, chrono et lingo (#82004). * J'y ai rapporté le template qui ajoute les dates de création du compte, du lien et de dernière connexion. * J'ai retiré les patchs qui corrigeaient les fixtures pour qu'on puisse les utiliser en même temps : on n'a pas d'appel à wcs et à authentic dans un même test. * J'ai déporté dans #83054 l'affichage des valeurs par défaut des gabarits utilisés dans les endpoints. exemple de ce qu'on listera via w.c.s. : ``` $ curl 'https://parsifal-passerelle.dev.publik.love/toulouse-maelis/test/get-link-list?NameID=83f6e19feb2043d2aafb041aea445b2c' | jq ".data[].text" "functest (lié le 02/11/2023)" "admin12 administre <nroche@yopmail.org> (lié le 26/01/2023 ; compte créé le 06/04/2020, dernière connexion le 02/11/2023)" ```
bdauvergne approved these changes 2023-11-06 11:15:31 +01:00
bdauvergne left a comment
Owner

Ok.

Ok.
nroche force-pushed wip/77295-parsifal-get-linked-users from a51d29393d to c212d3ff68 2023-11-10 10:21:30 +01:00 Compare
nroche merged commit 5cd1e3aacc into main 2023-11-10 10:38:06 +01:00
nroche deleted branch wip/77295-parsifal-get-linked-users 2023-11-10 10:38:06 +01:00
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/passerelle#239
No description provided.