Development #12910
corbo: provisionner le numéro de téléphone mobile et le mail de l'usager
0%
Fichiers
Révisions associées
Historique
Mis à jour par Serghei Mihai il y a plus de 7 ans
- Assigné à changé de Serghei Mihai à Jean-Baptiste Jaillet
Mis à jour par Serghei Mihai il y a environ 7 ans
- Assigné à changé de Jean-Baptiste Jaillet à Serghei Mihai
Mis à jour par Serghei Mihai il y a environ 7 ans
- Fichier 0001-add-corbo-privisioning-agent-12910.patch 0001-add-corbo-privisioning-agent-12910.patch ajouté
- Fichier 0001-add-corbo-agent-app-for-subscriptions-provisioning-1.patch 0001-add-corbo-agent-app-for-subscriptions-provisioning-1.patch ajouté
- Statut changé de Nouveau à En cours
- Patch proposed changé de Non à Oui
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 ?
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.
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.
Mis à jour par Serghei Mihai il y a environ 7 ans
- Fichier 0001-add-corbo-privisioning-agent-12910.patch 0001-add-corbo-privisioning-agent-12910.patch ajouté
Patch à jour (ma faute, j'ai demandé à JB de donner son avis)
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.
Mis à jour par Serghei Mihai il y a environ 7 ans
- Fichier 0001-add-corbo-privisioning-agent-12910.patch 0001-add-corbo-privisioning-agent-12910.patch ajouté
s/s/subscription/
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.
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 ?
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).
Mis à jour par Serghei Mihai il y a environ 7 ans
- Fichier 0001-add-corbo-privisionning-agent-12910.patch 0001-add-corbo-privisionning-agent-12910.patch ajouté
Ok.
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.
Mis à jour par Serghei Mihai il y a presque 7 ans
En suivant le même principe que l'agent dans bijoe.
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).
Mis à jour par Serghei Mihai il y a presque 7 ans
Ok, en effet.
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.
Mis à jour par Serghei Mihai il y a presque 7 ans
Bien vu, merci.
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)
Mis à jour par Paul Marillonnet il y a presque 7 ans
"super(CommonCommand, self)..."
et pourtant
"class Command"
?
Mis à jour par Serghei Mihai il y a presque 7 ans
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
?
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.
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 ?
Mis à jour par Serghei Mihai il y a presque 7 ans
Ce sont des clés obligatoires dans le message de notification
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.
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.
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.
Mis à jour par Serghei Mihai il y a presque 7 ans
Ok. Patch à jour.
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.
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.
Mis à jour par Serghei Mihai il y a presque 7 ans
Yep, il fallait un OR
.
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.
Mis à jour par Serghei Mihai il y a presque 7 ans
Corrigé et le test adapté.
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 ?
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é.
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()
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).
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 ?
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 ?
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à.
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.
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.
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.
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)
provisionning agent (#12910)