Produits Entr'ouvert » Corbo

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

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.

ack

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.

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

Ça reste plutôt court, s.

"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.

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 ?

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).

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.

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).

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

« 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)

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

Je me cache dans ma grotte...

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 ?

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.

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 ?

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

Ok pour moi.

Manque un test en fait.

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.

Ç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.

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.

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

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 ?

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()

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).

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

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

• les uuids connus
• les uuid == ''
• les uuid nul

C'est bon pour vous ?

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

Benjamin Dauvergne a écrit :

C'est bon pour vous ?

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

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

Et ack.