Produits Entr'ouvert » Passerelle

Copyright (C) 2020 : raté, on est en 2021 :)

Sépare avec une ligne vide les imports de Cryptodome et django, ça sera plus joli. Ajoute une ligne vide avant le début de la déclaration de CREATE_APPOINTMENT_SCHEMA pour faire un peu plus pep8.

Attention sur « 'needsConfirmation': {'type': 'boolean'}, », wcs ne sait pas envoyer de booléen facilement (sauf si ça correspond à une case à cocher dans le formulaire, ce dont je doute ici). Même chose pour les integer. Rappelle-toi que wcs ne sait envoyer que des strings : on voit dans ton test que CREATE_APPOINTMENT_PAYLOAD pourrait être délicat à construire par wcs.

« # error 500 if spaces are inserted » est un commentaire un peu inutile.

Pour la classe ESirius : dérive plutôt de HTTPResource qui va te faire gagner des méthodes d'accès HTTP gratuitement (même si y'en a pas à dans ce qu'on a testé, si ça se trouve y'en aura besoin ailleurs ou plus tard, genre un proxy ou un certif)

Ca s'écrit "eSirius" et pas "e-Sirius".

Appelle l'URL plutôt « ePlanning webservices URL » si jamais un jour on tape dans d'autres API eSirius et qu'il faudra indiquer d'autres URLs... Et mets un exemple avec https ;)

Il faut créer un self.request qui intégre le pre (ajout de l'auth) et le post (vérification du résultat). Ca fera des endpoints bien plus clairs. Intègres-y le calcul du token, il n'a rien à faire dans une fonction annexe qui ne sert jamais ailleurs.

Ce code là :

        if not post_data.get('user'):
            post_data.update({'user': {}})

        if 'user' not in post_data:
            post_data['user'] = {}

Le check est bizarre, il fait appel à un webservice qui n'est pas utilisé dans le reste du connecteur... mais en regardant le swagger je vois que y'a rien d'autre, so let it be.

Lors du delete, sur le « response.status_code == 304 » : ça n'a pas de sens ici (304 = Not Modified). Il faut demander à eSirius si c'est le résultat normal, s'ils n'ont pas plutôt confondu avec un habituel 404.

Dans le swagger on voit qu'il existe un GET /appointments​/{codeRDV} : ajoute-le dans les endpoints de ce connecteur, ça permettra (au moins) de vérifier ce qu'un POST a réussi à créer dans eSirius ; c'est important.

Dans le code on parle de "booking" alors qu'il s'agit chez eSirius de "appointments" : gardons ce terme. En fait je propose d'utiliser "id" au lieu de "booking_id" (ça sera moins verbeux) et de dire "Appointment not found" lors du 304.

A ce propos, pour la gestion du delete/304, ajoute un petit commentaire "# handle strange 304 delete response".

delete_appointment : retourne data:None plutôt qu'un dico vide.

Dans request, le moment où tu appelles requests.get avec "params=payload" est moche, un payload c'est pas des paramètres ;) J'aurais mis deux arguments distincts « def request(self, uri, method='get', params=None, json=None): », aussi parce que c'est plus proche de ce que requests gère.

Aussi, lors de l'appel à cette self.request, précise method='post' ou method='delete' plutôt que juste 'post' ou 'delete', ça permet de mieux comprendre ce que c'est que ces 'post' et 'delete', je trouve.

Enfin, juste pour faire mon top-pénible : le %s de strftime ce n'est pas portable, c'est du CPython post 1999... allez, passons, it works ;)

Bref, pas grand chose !

Sur self.request, le « raise APIError('Method Not Allowed', http_status=405) » : tu ne peux pas répondre ça au client qui t'appelle car ce n'est pas lui qui fait a fait l'erreur, c'est le code du connecteur. Il faudrait ici raiser un NotImplementedError...

Mais inutile de s'encombrer, fait juste :

# creation des headers
...
response = self.requests.request(method=method, url=url, headers=headers, params=params, json=json)
if method == 'delete' and response.status_code == 304:  # handle strange 304 delete response
    raise APIError('Appointment not found')
if response.status_code != 200:
    ...

Pour prévoir l'avenir et la possible disparition du token (sait on jamais vu les autres tickets), ne le génère que si secret_key existe.

Dans ton message de validation, ne dit pas "(longer keys are truncated)" puisque tu refuses les clés plus longues ;) Juste « DES key must be 8 bytes long »

Enfin pour "delete-appointment" je change d'avis (again) et te propose d'utiliser le verbe "delete". Parce que là on fait attends un POST avec un paramètre dans l'URL, sans payload, c'est tout bizarre, ça va être mal compris, éclaircissons. (et tant pis si pour l'instant la présentation n'est pas jolie dans le backoffice)

Nicolas Roche a écrit :

(remarques prises en comte).

Au cas où on souhaite pousser en recette un peu en avance,
en attendant d'avoir des réponses dans #51265 (j'ai forcé la clé à 8 octets minimum).

On attend pas, les clés feront 8 octets et rien d'autre ne marchera, tu pourrais même mettre min_length=8, max_length=8 sur le champ.

Benjamin Dauvergne a écrit :

On attend pas, les clés feront 8 octets et rien d'autre ne marchera, tu pourrais même mettre min_length=8, max_length=8 sur le champ.

+1 ... et ça t'évite la validation à la main, c'est mieux. A voir si ça se cumule avec blank=True, qui nous permettrait de dire "c'est soit un token de 8 octets pile, soit pas de token".

Nicolas Roche a écrit :

pour "delete-appointment" ... utiliser le verbe "delete"

fait, mais du coup j'ai également adapté le endpoint 'get-appointment' à la sauce REST.

Nope nope nope. Pour le get comme pour le delete, laisse id en paramètre classique. C'est comme ça que ça se configure dans les appels webservices de wcs.

Dernier point qui n'a pas été encore discuté.
Il me reste un soucis concernant #51500 : si l'on met une mauvaise clé (ou la bonne clé mais le mauvais "secret_id")
alors ESSI crash en 500 sur get_status et le connecteur est désactivé.

Comment ça, désactivé ? Ils coupent l'accès ??

ESSI crash en 500 sur get_status et le connecteur est désactivé.

Il s'agit de notre connecteur qui se met en stand-by, quand on met à jour les paramètres, je dirais ici :

class GenericCreateConnectorView(GenericConnectorMixin, CreateView):
...
    def form_valid(self, form):
...
        self.object.availability()

Nicolas Roche a écrit :

ESSI crash en 500 sur get_status et le connecteur est désactivé.

Il s'agit de notre connecteur qui se met en stand-by

Je ne comprends pas ce que tu dis. On n'a pas cette notion de "stand by". On a juste un truc qui dit "y'a un problème" et lance des alertes (mail) ; mais le connecteur est toujours là et utilisable.

Donc, pas de soucis, on aura des 500 en cas de pépin d'authentification... c'est comme ça. Ca ne devrait pas arriver souvent.

Avec les mauvais paramètres de connexion le connecteur est hors service, ça me semble tout à fait normal.