Project

General

Profile

Développement #45333

intégrer au niveau des connecteurs SMS la possibilité d'un envoi de test

Added by Frédéric Péters over 4 years ago. Updated over 4 years ago.

Status:
Fermé
Priority:
Bas
Assignee:
Target version:
-
Start date:
21 July 2020
Due date:
% Done:

0%

Estimated time:
Patch proposed:
Yes
Planning:
No

Description

Plutôt qu'avoir ça délégué à w.c.s.


Files


Related issues

Related to Passerelle - Développement #45814: Appliquer par defaut le template message_service_views aux connecteurs SMSFermé11 August 2020

Actions
Related to Passerelle - Développement #45815: Déplacer le template message_service_view dans le répertoire SMSFermé11 August 2020

Actions
Related to Passerelle - Développement #45816: Exclure les attributs sensibles des champs décrivants les connecteurs SMSFermé11 August 2020

Actions
Related to Passerelle - Développement #45817: Afficher les attributs des connecteurs SMS dans le template message_service_viewFermé11 August 2020

Actions
Related to Passerelle - Développement #45829: Ajouter une description au endpoint send des connecteurs SMSFermé11 August 2020

Actions
Related to Passerelle - Développement #46611: Permettre de charger les fichiers urls depuis une application commune, tout en gardant les fonctionnalités liées aux connecteurs qui dérivent de cette application.Fermé14 September 2020

Actions

Associated revisions

Revision 6ff2dc43 (diff)
Added by Nicolas Roche over 4 years ago

views: explicitly get form_class from generic connector mixin (#45333)

Revision 257334fd (diff)
Added by Nicolas Roche over 4 years ago

sms: add a button to test sending sms (#45333)

History

#1

Updated by Nicolas Roche over 4 years ago

  • Related to Développement #34067: test de sms : ça affiche "Le SMS a été envoyé correctement." alors que non added
#2

Updated by Nicolas Roche over 4 years ago

  • Related to Développement #44815: avoir la possibilité d'ajouter un job en lui demandant de s'exécuter tout de suite added
#3

Updated by Nicolas Roche over 4 years ago

Patch qui reprend de l'ancien code avant le passage aux envois asynchrone : https://dev.entrouvert.org/issues/21465

#4

Updated by Frédéric Péters over 4 years ago

L'objectif ici n'est pas d'avoir un endpoint d'envoi synchrone.

Il s'agit d'avoir un bouton "Tester l’envoi" qui ouvre une popup et permette à l'admin de taper un numéro de destination et un texte puis de valider le formulaire, l'envoi se fait et la page s'affiche avec l'info "un envoi vient d'être fait" (django.contrib.messages).

#5

Updated by Nicolas Roche over 4 years ago

  • Status changed from Solution proposée to Nouveau

(compris)

#6

Updated by Nicolas Roche over 4 years ago

  • Related to deleted (Développement #34067: test de sms : ça affiche "Le SMS a été envoyé correctement." alors que non)
#7

Updated by Nicolas Roche over 4 years ago

  • Related to deleted (Développement #44815: avoir la possibilité d'ajouter un job en lui demandant de s'exécuter tout de suite)
#8

Updated by Nicolas Roche over 4 years ago

Pour info, ici j'envisagerais de partir sur un chemin de croix (cf mon patch fourre-tout) :
  • uniformiser l'utilisation du template messages_services_views pour tous les connecteurs SMS
  • le déplacer dans /sms/template
  • ajouter le lien vers une popup dedans
  • ajouter des fichiers urls.py, views.py et forms.py dans /sms
  • bidouiller base/__init__.py pour charger sms/urls.py sur les connecteurs SMS
  • bidouiller une fonction get_instance() dans la vue pour retrouver l'objet connecteur
Et là Thomas me dit que je pourrais éviter tout ça :
  • permettre d'appeler le endpoint de façon synchrone
  • ajouter mon formulaire directement dans le template messages_services_views
  • faire en sorte que le formulaire envoie ses paramètres en JSON sur le endpoint

Bref, je préfère passer mon chemin pour le moment quitte à revenir plus tard.

#9

Updated by Nicolas Roche over 4 years ago

  • Related to Développement #45814: Appliquer par defaut le template message_service_views aux connecteurs SMS added
#10

Updated by Nicolas Roche over 4 years ago

  • Related to Développement #45815: Déplacer le template message_service_view dans le répertoire SMS added
#11

Updated by Nicolas Roche over 4 years ago

  • Related to Développement #45816: Exclure les attributs sensibles des champs décrivants les connecteurs SMS added
#12

Updated by Nicolas Roche over 4 years ago

  • Related to Développement #45817: Afficher les attributs des connecteurs SMS dans le template message_service_view added
#13

Updated by Nicolas Roche over 4 years ago

#14

Updated by Nicolas Roche over 4 years ago

  • Related to Développement #46611: Permettre de charger les fichiers urls depuis une application commune, tout en gardant les fonctionnalités liées aux connecteurs qui dérivent de cette application. added
#15

Updated by Nicolas Roche over 4 years ago

Un bouton (oups, un lien en fait) "Tester l’envoi" qui ouvre une popup.

#16

Updated by Nicolas Roche over 4 years ago

  • Assignee set to Nicolas Roche
#17

Updated by Valentin Deniaud over 4 years ago

get_instance ne peut pas fonctionner comme ça (capitalize()), il faut utiliser GenericConnectorMixin qui te donne self.model. Par contre le formulaire ne doit pas être écrasé, donc il faut patcher le mixin avec genre

-        self.form_class = self.model.get_manager_form_class(exclude=self.exclude_fields)
+        if not hasattr(self, 'form_class'):
+            self.form_class = self.model.get_manager_form_class(exclude=self.exclude_fields)

L'appel à send_sms n'est pas le même que celui qui est fait par l'envoi normal via BaseSMSResource, il manque le paramètre stop. Ça fait crasher l'envoi avec le connecteur OVH par ex, qui fait un kwargs['stop'] (le test passe parce que tu mock la fonction d'envoi). Bref, juste ajouter un stop=False.

À l'usage, j'aurais aimé avoir un bouton « Resend test message », parce que remplir le formulaire de test, valider l'envoi, voir que ça ne marche pas, corriger, remplir le formulaire de test avec les mêmes infos, valider l'envoi, etc, c'est un peu relou.
Genre au moment de la redirection vers instance.get_absolute_url(), ajouter des paramètres d'URL correspondant à ceux du formulaire, puis sur la page du connecteur avoir un formulaire vers SmsTestSendView avec que des champs cachés ayant pour valeur request.GET.<champ>, et le bouton de soumission qui s'appelle « Resend test message ». Je te laisse voir si c'est faisable, sinon pas grave.

#18

Updated by Nicolas Roche over 4 years ago

get_instance ne peut pas fonctionner comme ça (capitalize()), il faut utiliser GenericConnectorMixin qui te donne self.model.

merci. J'ai dû surcharger init_stuff (avec le patch du mixin proposé les tests plantent sur le connecteur CSV).

L'appel à send_sms n'est pas le même que celui qui est fait par l'envoi normal via BaseSMSResource, il manque le paramètre stop. Ça fait crasher l'envoi avec le connecteur OVH par ex. Bref, juste ajouter un stop=False.

merci, je n'avais pas détecté le soucis en m'envoyant les SMS avec twilio.

       if not kwargs['stop']:   # yep, crash ici
            params.update({'noStop': 1})  # yep, je met stop=False

rq: https://dev.entrouvert.org/issues/47034#note-1
noStop: 1 pour ne pas afficher "STOP au XXXXX" à la fin du message pour un SMS à caractère non commercial

À l'usage, j'aurais aimé avoir un bouton « Resend test message », parce que remplir le formulaire de test, valider l'envoi, voir que ça ne marche pas, corriger, remplir le formulaire de test avec les mêmes infos, valider l'envoi, etc, c'est un peu relou.

Moi je veux bien, mais c'est pas un peu over-engenering (le navigateur se souvient des valeurs, ça fait 7 clics) ?

#19

Updated by Valentin Deniaud over 4 years ago

Nicolas Roche a écrit :

get_instance ne peut pas fonctionner comme ça (capitalize()), il faut utiliser GenericConnectorMixin qui te donne self.model.

merci. J'ai dû surcharger init_stuff (avec le patch du mixin proposé les tests plantent sur le connecteur CSV).

Et si tu essayais d'ajouter get_form_class à GenericConnectorMixin, à la place d'affecter un self.form_class autoritaire, et ensuite surcharger cette nouvelle méthode dans SmsTestSendView, ça permettrait de remettre ce bazar dans les clous de django (https://docs.djangoproject.com/en/3.1/ref/class-based-views/mixins-editing/#django.views.generic.edit.FormMixin.get_form_class).

#20

Updated by Nicolas Roche over 4 years ago

Oui, je ne voulais pas toucher au code du Mixin mais c'est plus propre.

#21

Updated by Valentin Deniaud over 4 years ago

Tu dois pouvoir enlever l'affectation explicite de self.form_class dans le mixin, et en fait pas besoin de surcharger get_form_class dans le formulaire SMS, l'attribut statique devrait suffire (le problème initial étant qu'il était écrasé par la précédente affectation).

#22

Updated by Nicolas Roche over 4 years ago

Oui et non.

Tu dois pouvoir enlever l'affectation explicite de self.form_class dans le mixin

oui à condition de remplacer l'endroit où cette affectation est utilisée (dans GenericConnectorView) -> 0001

et en fait pas besoin de surcharger get_form_class dans le formulaire SMS, l'attribut statique devrait suffire

non, il faut de toute façon redéfinir get_form_class car elle est appelée par le FormMixin de django.

#23

Updated by Valentin Deniaud over 4 years ago

  • Status changed from Solution proposée to Solution validée
#24

Updated by Nicolas Roche over 4 years ago

  • Status changed from Solution validée to Résolu (à déployer)
commit 257334fdc5354773797297eb69ece1ffaac332bb
Author: Nicolas ROCHE <nroche@entrouvert.com>
Date:   Wed Sep 16 16:23:49 2020 +0200

    sms: add a button to test sending sms (#45333)

commit 6ff2dc4365c8511e083ee6c16ee6b91ea691224a
Author: Nicolas ROCHE <nroche@entrouvert.com>
Date:   Thu Oct 15 14:52:35 2020 +0200

    views: explicitly get form_class from generic connector mixin (#45333)
#25

Updated by Frédéric Péters over 4 years ago

  • Status changed from Résolu (à déployer) to Solution déployée

Also available in: Atom PDF