Produits Entr'ouvert » Passerelle

La lecture du patch me fait dire qu'actuellement c'était la première annexe qui était prise, non ?

À faire évoluer ça, à ajouter un paramètre permettant de prendre l'annexe, j'en ferais un paramètre acceptant une valeur permettant de pointer l'annexe souhaitée.

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

La lecture du patch me fait dire qu'actuellement c'était la première annexe qui était prise, non ?

Tout à fait.

À faire évoluer ça, à ajouter un paramètre permettant de prendre l'annexe, j'en ferais un paramètre acceptant une valeur permettant de pointer l'annexe souhaitée.

Ok, et si elle n'existe pas une erreur 404.

Plutôt annex que annexe, pour suivre l'anglais dans nos enpoints.

Et coller le except ValueError au int(annexe) (faire deux try/except quoi)

On dit plutôt "appendix" en vrai anglais.

On a vraiment besoin de vrais fichiers dans tests/data ?

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

On a vraiment besoin de vrais fichiers dans tests/data ?

Ce n'est bien sûr pas une question, et la réponse est "non".

Quand j'écrivais "vrai fichier" je pensais en fait même qu'on n'avait pas besoin de fichier du tout; parce qu'à regarder les tests je voyais notamment :

        file_data = file(os.path.join(os.path.dirname(__file__), 'data',
                                      filename))
        base64_data = base64.b64encode(file_data.read())

        data = {'type': typ, 'subtype': subtyp, 'visibility': visibility,
                'title': title, 'data': base64_data, 'content-type':'application/pdf'}
...
        assert req.find('ns1:DocumentPrincipal', SOAP_NAMESPACES).text == base64_data

Ça marcherait très bien avec juste base64_data = 'aGVsbG8gd29ybGQ='.

L'autre lieu, touché par ce patch, c'est autour des lignes :

    file_sent = os.path.join(os.path.dirname(__file__), 'data/iparapheur_test.pdf')
    assert resp.headers['Content-Type'] == 'application/pdf'
    assert 'filename=test.pdf' in resp.headers['Content-Disposition']
    assert hashlib.md5(resp.body[:8192]).hexdigest() == hashlib.md5(file(file_sent).read()[:8192]).hexdigest()

Et pareil, j'accorde iparapheur_get_file_response.xml parce que c'est moche comme tout (et moche comme tout^2 d'appeler .xml un fichier qui n'en est pas), mais ici aussi, plutôt qu'un vrai fichier posé sur le filesystem pour file_sent, ça pourrait juste être hashlib.md5('Hello world').hexdigest().

        response['Content-Disposition'] = 'inline; filename=%s.pdf' % fichier_nom

cf #16250

    @endpoint(serializer_type='json-api', perm='can_access', name='get-file', pattern='(?P<file_id>[\w-]+)')

À rebaser parce qu'il n'y a désormais plus de serializer_type.

-    for filename in ('iparapheur_test.odt', 'iparapheur_test.pdf'):
+    for filename in ('iparapheur_test.odt', 'iparapheur_doc.pdf'):

Ce passage est curieux, le patch modifie un comportement pour le cas qui était traité dans ce test ?

    assert 'filename=test.pdf' in resp.headers['Content-Disposition']
...
    assert 'filename=test.pdf' in resp.headers['Content-Disposition']

J'étais curieux de voir d'où sortait ce nom de fichier,

<MetaDonnees><MetaDonnee><nom>ph:dossierTitre<<nom><valeur>test</valeur></MetaDonnee></MetaDonnees>

C'est tiré de là, sorti ainsi :

filename = '%s.pdf' % resp.MetaDonnees.MetaDonnee[0]['valeur']

Ça me semble quand même des plus hasardeux de sortir ça ainsi, sans se soucier du tout que le premier élément soit bien "ph:dossierTitre".

Et mettre .pdf de force, plutôt que se baser sur xmime:contentType="application/pdf", une erreur.

Et pour l'annexe récupérée dans le dernier test, étant donné que :

<DocumentsAnnexes><DocAnnexe><nom>iParapheur_impression_dossier.pdf</nom><fichier xmime:contentType="application/pdf">

Je me dis que ce n'est pas correct d'avoir là test.pdf, que ça devrait être le nom dans <nom>.

À passer de fichier_nom à filename, en profiter aussi pour ne pas avoir de variable nommée "fichier" (suggestion ici : "document") ?

À part ça je pense que c'est ok.

ok