Projet

Général

Profil

Development #12910

corbo: provisionner le numéro de téléphone mobile et le mail de l'usager

Ajouté par Serghei Mihai il y a plus de 7 ans. Mis à jour il y a plus de 6 ans.

Statut:
Fermé
Priorité:
Normal
Assigné à:
Version cible:
-
Début:
24 août 2016
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Oui
Planning:

Fichiers

0001-add-corbo-privisioning-agent-12910.patch (4,43 ko) 0001-add-corbo-privisioning-agent-12910.patch Serghei Mihai, 10 mars 2017 17:09
0001-add-corbo-agent-app-for-subscriptions-provisioning-1.patch (654 octets) 0001-add-corbo-agent-app-for-subscriptions-provisioning-1.patch Serghei Mihai, 10 mars 2017 17:10
0001-add-corbo-privisioning-agent-12910.patch (4,33 ko) 0001-add-corbo-privisioning-agent-12910.patch Serghei Mihai, 16 mars 2017 17:48
0001-add-corbo-privisioning-agent-12910.patch (4,41 ko) 0001-add-corbo-privisioning-agent-12910.patch Serghei Mihai, 17 mars 2017 09:53
0001-add-corbo-privisionning-agent-12910.patch (4,74 ko) 0001-add-corbo-privisionning-agent-12910.patch Serghei Mihai, 19 mars 2017 00:29
0001-provisionning-agent-12910.patch (3,92 ko) 0001-provisionning-agent-12910.patch Serghei Mihai, 04 mai 2017 00:07
0001-provisionning-agent-12910.patch (4,05 ko) 0001-provisionning-agent-12910.patch Serghei Mihai, 04 mai 2017 17:22
0001-provisionning-agent-12910.patch (4,09 ko) 0001-provisionning-agent-12910.patch Serghei Mihai, 18 mai 2017 16:11
0001-provisionning-agent-12910.patch (4,07 ko) 0001-provisionning-agent-12910.patch Serghei Mihai, 01 juin 2017 11:00
0001-provisionning-agent-12910.patch (12,1 ko) 0001-provisionning-agent-12910.patch Serghei Mihai, 08 juin 2017 16:20
0001-provisionning-agent-12910.patch (11,7 ko) 0001-provisionning-agent-12910.patch Serghei Mihai, 08 juin 2017 18:19
0001-provisionning-agent-12910.patch (12 ko) 0001-provisionning-agent-12910.patch Serghei Mihai, 09 juin 2017 11:46
0001-provisionning-agent-12910.patch (12 ko) 0001-provisionning-agent-12910.patch Serghei Mihai, 12 juin 2017 11:07
0001-provisionning-agent-12910.patch (12 ko) 0001-provisionning-agent-12910.patch Serghei Mihai, 15 juin 2017 11:24
0001-provisionning-agent-12910.patch (12 ko) 0001-provisionning-agent-12910.patch Serghei Mihai, 15 juin 2017 17:00
0001-provisionning-agent-12910.patch (12,2 ko) 0001-provisionning-agent-12910.patch Serghei Mihai, 16 juin 2017 10:47
0001-provisionning-agent-12910.patch (12,5 ko) 0001-provisionning-agent-12910.patch Serghei Mihai, 06 juillet 2017 19:57

Révisions associées

Révision e5b05b4d (diff)
Ajouté par Serghei Mihai il y a plus de 6 ans

provisionning agent (#12910)

Historique

#1

Mis à jour par Serghei Mihai il y a plus de 7 ans

  • Assigné à changé de Serghei Mihai à Jean-Baptiste Jaillet
#2

Mis à jour par Serghei Mihai il y a environ 7 ans

  • Assigné à changé de Jean-Baptiste Jaillet à Serghei Mihai
#4

Mis à jour par Benjamin Dauvergne il y a environ 7 ans

Pour les mobiles on part du principe que le format aura été vérifié coté authentic donc ?

#5

Mis à jour par Serghei Mihai il y a environ 7 ans

C'est mon idée.
Il me semble qu'il y avait une demande d'avoir des validateurs sur les champs du profil de l'usager, notamment sur les numéros de téléphone.

#6

Mis à jour par Jean-Baptiste Jaillet il y a environ 7 ans

ack

#7

Mis à jour par Frédéric Péters il y a environ 7 ans

En live tout à l'heure, interrogation sur le sens du atomic() avec une seule action dans le bloc, et commentaire sur les noms de variable à un caractère.

#8

Mis à jour par Serghei Mihai il y a environ 7 ans

Patch à jour (ma faute, j'ai demandé à JB de donner son avis)

#9

Mis à jour par Frédéric Péters il y a environ 7 ans

for s in Subscription.objects.filter(uuid=obj['uuid']):

Ça reste plutôt court, s.

#11

Mis à jour par Frédéric Péters il y a environ 7 ans

"privisioning" dans le message de commit.

Il faudrait faire le traitement uniquement si la notif concerne des utilisateurs, regarder dans notification['objects']['@type'].

Idéalement aussi prendre en compte la présence de notification['full'] et dans ce cas retirer les notifications d'usagers qui n'apparaitraient pas dans le message.

#12

Mis à jour par Serghei Mihai il y a environ 7 ans

Frédéric Péters a écrit :

Idéalement aussi prendre en compte la présence de notification['full'] et dans ce cas retirer les notifications d'usagers qui n'apparaitraient pas dans le message.

Pour être sur de bien comprendre: le notification["full"] est toujours à False pour les utilisateurs. C'est dans le cas d'un True qu'il faut retirer les abonnements des usagers n'étant pas dans le message ?

#13

Mis à jour par Frédéric Péters il y a environ 7 ans

Pour être sur de bien comprendre: le notification["full"] est toujours à False pour les utilisateurs.

Aujourd'hui. C'est pour ça que j'écrivais "idéalement". Parce qu'idéalement aussi, on aurait une commande pour relancer un provisionning "full" depuis authentic (genre après que rabbitmq aie été down plusieurs heures).

#15

Mis à jour par Frédéric Péters il y a presque 7 ans

En mode "full" tu ne mets pas à jour les données ?

obj['mobile'], ça existe assuré toujours ? (réponse : non), et quand il est présent mais None ou vide, plutôt retirer l'objet subscription.

Aussi, la tendance posée par Benjamin c'est de déplacer les agents vers leurs modules respectifs (cf bijoe et #13325); ça permet aussi de plus simplement écrire des tests, pas besoin pour hobo de dépendre de tous les modules.

#16

Mis à jour par Serghei Mihai il y a presque 7 ans

En suivant le même principe que l'agent dans bijoe.

#17

Mis à jour par Frédéric Péters il y a presque 7 ans

  • Projet changé de Hobo à Corbo
#18

Mis à jour par Benjamin Dauvergne il y a presque 7 ans

Pour moi il y a toujours le problème signalé par Fred que si le profile ne contient pas de champ mobile on va avoir une KeyError, il faut faire un mobile = obj.get('mobile') puis tester cette valeur plutôt que de tester directement obj['mobile'], pour email je pense qu'on est plus à l'aise (mais je ne pense pas que ça coûte cher de faire pareil).

#20

Mis à jour par Benjamin Dauvergne il y a presque 7 ans

Manque un continue après le premier subscription.delete(), ça va faire delete() puis save(), pas bon.

#22

Mis à jour par Thomas Noël il y a presque 7 ans

« full = notification['full'] if 'full' in notification else False » ne sert à rien, en ligne 32 il suffit de faire un « if notification.get('full'): »

le Subscription.uuid peut être blank selon le modèle, donc le Subscription.objects.exclude(uuid__in=uuids).delete() va effacer tout ceux-là, à mon avis faut les exclure.

Je trouve que "data" pourrait s'appeler "users" et "obj" s'appeler "user", ça simplifierait la compréhension.

(c'est tout pour aujourd'hui)

#23

Mis à jour par Paul Marillonnet il y a presque 7 ans

"super(CommonCommand, self)..."
et pourtant
"class Command"
?

#24

Mis à jour par Serghei Mihai il y a presque 7 ans

Je me cache dans ma grotte...

#26

Mis à jour par Paul Marillonnet il y a presque 7 ans

Thomas Noël a écrit :

le Subscription.uuid peut être blank selon le modèle, donc le Subscription.objects.exclude(uuid__in=uuids).delete() va effacer tout ceux-là, à mon avis faut les exclure.

Un UUID blank ??? Ne faut-il pas justement modifier le modèle au lieu d'ajouter un filtre uuid__isnull=False ?

#27

Mis à jour par Serghei Mihai il y a presque 7 ans

Paul Marillonnet a écrit :

Un UUID blank ??? Ne faut-il pas justement modifier le modèle au lieu d'ajouter un filtre uuid__isnull=False ?

Non, car il est possible d'abonner des mails à une categorie à partir d'un csv. Pour ceux-là, bien évidemment, il n'y aura pas de provisionning.

#28

Mis à jour par Paul Marillonnet il y a presque 7 ans

object_type = notification['objects']['@type']
...
users = notification['objects']['data']
action = notification['@type']

C'est 'KeyError-proof' ? Ca va pas casser l'appli si jamais certaines entrées du dict notification sont manquantes ?

#29

Mis à jour par Serghei Mihai il y a presque 7 ans

Ce sont des clés obligatoires dans le message de notification

#30

Mis à jour par Paul Marillonnet il y a presque 7 ans

Ok pour moi.

#31

Mis à jour par Benjamin Dauvergne il y a presque 7 ans

Manque un test en fait.

#32

Mis à jour par Serghei Mihai il y a presque 7 ans

Les voici, en m'inspirant des tests de provisionning de l'agent authentic dans hobo.

#33

Mis à jour par Serghei Mihai il y a presque 7 ans

Et en gérant les cas ou le uuid peut être une chaîne vide.

#34

Mis à jour par Benjamin Dauvergne il y a presque 7 ans

Le test de deprovisionning ne teste rien d'intéressant, il faudrait vérifier que les souscriptions pour les uuids dans le message de deprovisionning ont disparu. Sur le notify full je vérifierai en plus du décompte qu'il n'y a plus d'uuid == ''. Sur les notifications mobile/email only, je vérifierai que pour les uuids du message, on a plus de souscription de type mailto ou sms selon le cas.

#36

Mis à jour par Serghei Mihai il y a presque 7 ans

Comme remarqué par Fréd sur le salon tech:

user['mobile'] = '0(mobile)s' % user (← erreur)

C'est corrigé dans le patch joint.

#37

Mis à jour par Benjamin Dauvergne il y a presque 7 ans

Ça c'est bizarre:

Subscription.objects.filter(uuid__isnull=False).filter(uuid='').exclude(uuid__in=uuids).delete()

Ça ne va supprimer que uuid='', ça devrait se voir dans le test full, sinon faut voir pourquoi.

#38

Mis à jour par Serghei Mihai il y a presque 7 ans

Yep, il fallait un OR.

#39

Mis à jour par Benjamin Dauvergne il y a presque 7 ans

Je ne comprends toujours pas :)

Subscription.objects.filter(Q(uuid__isnull=False)|Q(uuid='')).exclude(uuid__in=uuids).delete()

Ça veut dire de supprimer toutes les souscriptions qui ont un uuid ou un uuid vide (uuid vide != NULL, donc c'est déjà compris dans la première condition il me semble), sauf pour les uuids connus.

#40

Mis à jour par Serghei Mihai il y a presque 7 ans

En fait le exclude suffit, que suis-je bête.

#41

Mis à jour par Serghei Mihai il y a presque 7 ans

Corrigé et le test adapté.

#42

Mis à jour par Thomas Noël il y a presque 7 ans

Heu... là, on revient à ma remarque d'il y a cinq jours sur « Subscription.objects.exclude(uuid__in=uuids).delete() » qui va supprimer tous les abonnements pour les uuid blank. C'est pas bon.

Subscription.objects.exclude(uuid__in=uuids).exclude(uuid='').exclude(uuid_isnull=True).delete() non ?

#43

Mis à jour par Serghei Mihai il y a presque 7 ans

Pf... un peu perdu dans les blank et null. Patch à jour avec la suggestion de Thomas et test adapté.

#44

Mis à jour par Benjamin Dauvergne il y a presque 7 ans

Thomas Noël a écrit :

Heu... là, on revient à ma remarque d'il y a cinq jours sur « Subscription.objects.exclude(uuid__in=uuids).delete() » qui va supprimer tous les abonnements pour les uuid blank. C'est pas bon.

Subscription.objects.exclude(uuid__in=uuids).exclude(uuid='').exclude(uuid_isnull=True).delete() non ?

Il faut faire un Q()|Q()|... car les exclude() se combine par défaut avec AND comme les filtres (l'idée étant que .exclude(cond1).exclude(cond2) soit équivalent à .exclude(cond1, cond2)). Donc ici:

Subscription.objects.exclude(Q(uuid__in=uuids)|Q(uuid='')|Q(uuid_isnull=True)).delete()
#45

Mis à jour par Serghei Mihai il y a presque 7 ans

Je ne veux pas supprimer des abonnements qui n'ont pas de uuid ou uuid égal à une chaîne vide car ça peut être des abonnements faits via le backoffice en important un csv avec les adresses mail (fait pour Meyzieu).

#46

Mis à jour par Frédéric Péters il y a plus de 6 ans

Les personnes qui ont suivi peuvent regarder une dernière fois et valider ou pas ?

#47

Mis à jour par Benjamin Dauvergne il y a plus de 6 ans

Subscription.objects.exclude(Q(uuid__in=uuids)|Q(uuid='')|Q(uuid_isnull=True)).delete()
Je ré-explique, donc on supprime tout sauf:
  • les uuids connus
  • les uuid == ''
  • les uuid nul

C'est bon pour vous ?

#48

Mis à jour par Benjamin Dauvergne il y a plus de 6 ans

La condition isnull est mal écrite dans mon code, mais l'idée est là.

#49

Mis à jour par Thomas Noël il y a plus de 6 ans

Benjamin Dauvergne a écrit :

C'est bon pour vous ?

Ouaip ; donc y'a quelque chose qui ne va pas dans les tests.

#50

Mis à jour par Serghei Mihai il y a plus de 6 ans

Ok.
Comme les CharField nuls et chaînes vides sont écrits dans la base comme des chaînes vides (https://docs.djangoproject.com/en/1.8/ref/models/fields/#null) et que dans les futures versions ça va écrire du "null" (https://code.djangoproject.com/ticket/4136) dans les tests je vérifie les 2 cas.

#51

Mis à jour par Thomas Noël il y a plus de 6 ans

Renommer tests/test_notify.py en tests/test_hobo_notify.py pour plus de clarté.

Et ack.

#52

Mis à jour par Serghei Mihai il y a plus de 6 ans

  • Statut changé de En cours à Résolu (à déployer)

Rénommé et poussé:

commit e5b05b4db05d0a69f19ea4751ad25c8e0b33274d (origin/master)
Author: Serghei Mihai <smihai@entrouvert.com>
Date:   Fri Apr 21 11:43:59 2017 +0200

    provisionning agent (#12910)

#53

Mis à jour par Serghei Mihai il y a plus de 6 ans

  • Statut changé de Résolu (à déployer) à Fermé

Formats disponibles : Atom PDF