Produits Entr'ouvert » Passerelle

Poussé dans la branche teamnet

Je veux bien une relecture des patchs dans la branche teamnet si quelqu'un peut

J'étais parti pour suggérer l'utilisation de ET.SubElement, ce qui éviterait déjà une belle série de .append() mais je crains que la génération XML reste quand même plutôt illisible et qu'il faille plutôt aborder les choses autrement.

Il existe django-xslt mais c'est peut-être un peu aventureux (et qui veut faire de l'XSLT…) et donc, même si ce n'est vraiment pas orthodoxe, je suggérerais qu'on utilise des templates pour faire le travail ici. On pourrait alors voir pour s'intégrer dans le "system checks framework" de Django pour avoir une validation des fichiers XML au démarrage ou alors au runtime, dans le get_data() la chaîne contenant de l'XML serait passée dans ElementTree pour vérification.

À part ça, pay_invoice(), il n'y a pas de retour mais InvoicePayView fait un return.

On doit pouvoir se calmer sur les **kwargs inutilisés, ils n'arrangent pas la lisibilité et ne facilient pas le debug.

Quand on fait get_data(request, '12', **kwargs), le '12' apparaitrait moins arbitraire s'il était passé comme months=12. (et en entier, pas chaîne).

Je ne vos pas le le problème avec cette façon de générer du XML, en plus c'est pas très long là et +1 pour le SubElement. Si il y a une espèce de régularité entre les appels on peut peut-être factorisé l'entête du message mais bon sinon... Je n'aime pas quand la moitié du code et dans des templates en fait (on parle pas de présentation là, une fois que ça marchera ça ne bougera jamais plus).

ok, partons sur des ET.SubElement, de manière un peu compacte, genre :

invoice_xml = ET.Element('FACTUREPDF')
ET.SubElement(invoice_xml, 'IDFAMILLE').text = famille_id
ET.SubElement(ET.SubElement(invoice_xml, 'FACTURES'), 'NOFACTURE').text = invoice

(plutôt que l'existant

        invoice_xml = ET.Element('FACTUREPDF')
        family_xml = ET.Element('IDFAMILLE')
        family_xml.text = family_id
        invoice_xml.append(family_xml)
        invoices_xml = ET.Element('FACTURES')
        no = ET.Element('NOFACTURE')
        no.text = invoice
        invoices_xml.append(no)
        invoice_xml.append(invoices_xml)

J'y pensais plus mais il y a xml.etree.ElementTree.TreeBuilder aussi:

tb = TreeBuilder()
tb.start('FACTUREPDF', {})
tb.start('IDFAMILLE', {})
tb.data(family_id)
tb.end()
tb.start('FACTURES', {})
tb.start('NOFACTURE', {})
tb.data(invoice)
tb.end()
tb.end()
invoice_xml = tb.close()

À noter qu'un fois qu'on a un builder c'est très facile de décrire un schéma XML avec des S-expression, genre:

def generate_xml(schema):
    tb = TreeBuilder()
    def helper(schema):
        tag = schema[0]
        attribs = schema[1] if len(schema) > 0 and isinstance(schema[1], dict) else None
        tb.start(tag, attribs or {}
        if attribs is None:
            children = schema[1:]
        else:
            children = schema[2:]
        for child in children:
            if isinstance(child, basestring):
                tb.data(child)
            else:
                helper(child)
        helper(schema)
        return tb.close()

generate_xml(('FACTUREPDF',
  ('IDFAMILLE', family_id),
  ('FACTURES',
    ('NOFACTURE', invoice))))

Je crois me souvenir qu'on appellait ce format S-XML dans le temps ou je faisais du LISP.

Serghei Mihai a écrit :

0001-contrib.teamnet_axel-xml-tree-building-refactored.patch : Refactorisation avec SubElement

Ack pour celui-ci.

sur 0001-contrib.teamnet_axel-past-and-active-invoices-retrie.patch:

• invoice['id'] doit contenir l'id au sens "passerelle", et pour axel ça sera familyid-factureid
• éventuellement ajoute un invoice['display_id'] qui sera utilisable dans le template pour afficher le numéro "utilisateur" (et pas technique)

pour ne plus voir de traces des vielles choses, j'aurais renommé les méthodes InvoicesView et HistoryInvoicesView et leur nom teamnet-axel-invoices et teamnet-axel-invoices-history

• ajouter un commentaire dans get_invoices lors du "operation = 'HistoriqueFacturesRegie'" qui explique que cette operation renvoie toutes les factures sur x mois, mais aussi les futures.
• et donc, ajouter commentaire dans la vue historique pour dire qu'on est obligés de retirer les nouvelles de l'historique pour avoir un vrai historique (pfffff)
• supprimer le mixin InvoicesMixin qui ne sert à rien qu'à embrouiller le code, appeler directement get_invoices dans les vues invoices et invoices/history

retirer le month=12 dans le get_data de invoices/history parce que ça sera toujours 12 (non paramétrable)

0002-contrib.teamnet_axel-invoice-details-endpoint-8035.patch :

• retirer le mixin inutilisé
• mettre un petit commentaire pour mémoire qui explique que le code est un peu bête parce que teamnet ne propose pas de webservice pour chercher une facture isolés (on doit passer par get_invoices sur 12 mois)

Je ne suis pas sûr que les bons patchs aient été attachés, au moins les commentaires suivant ont été ignorés :

• retirer le month=12 dans le get_data de invoices/history parce que ça sera toujours 12 (non paramétrable)
• mettre un petit commentaire pour mémoire qui explique que le code est un peu bête

(pas regardé davantage)

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

• retirer le month=12 dans le get_data de invoices/history parce que ça sera toujours 12 (non paramétrable)

Je crois que ça parlait de

class PastInvoicesView(InvoicesMixin, FamilyView):
    def get_data(self, request, regie_id, months=12, **kwargs)

• mettre un petit commentaire pour mémoire qui explique que le code est un peu bête

# exclude the active invoices from the past ones
active = self.object.get_invoices(regie_id, family_id)