Un manque de sérieux dans l’évolution du framework depuis quelque temps ?

Bonjour à tous,

Cela fait un moment que je n’avais pas posté de billet. Depuis l’été dernier la période a été très chargée pour moi du fait de la fin de mes études et de divers travaux sur lesquels j’aurais j’espère l’occasion de revenir plus tard.

Comme certains d’entre vous me l’ont déjà entendu dire, je me réclame d’être un « consultant fonctionnel », c’est-à-dire :

-Capable de comprendre et recueillir les besoins de l’entreprise utilisatrice
-Fort de cette compréhension et de celle des principaux modules d’OpenERP, je suis censé trouver la manière la plus pérenne pour développer une fonctionnalité et la modéliser avant d’envoyer en développement.
-Je suis capable de faire des développements basiques, comme rajouter un nouveau champ, adapter une vue, un droit d’accès, corriger une traduction…. Bref tout ce qui est pris en charge par le framework car celui-ci est censé gérer les besoins les plus courants de l’ERPs. Cela permet de soumettre l’interface au client dès le début du projet ce qui est un atout extrêmement efficace pour gagner du temps sur le projet.
-Je ne suis pas capable de faire du pur développement python. Je suis néanmoins capable de le lire et de faire parfois quelques corrections mineures.

Pourquoi je présente cette définition du consultant fonctionnel ? Car je pense que n’importe qui, qui a une expertise métier, peut devenir un consultant fonctionnel sur OpenERP, cela prendra juste du temps et au final vous ne serez bloqué que par les développements pointus.

C’est possible car depuis le départ, OpenERP a mis en place le framework OpenObject qui gère les objets en base de donnée, les menus, les vues, les droits d’accès, etc… Rendant pratiquement possible au premier venu de faire des modifications majeures sur le fonctionnement de l’ERP. Cela a toujours été la principale force d’OpenERP ! Sa simplicité d’accès au code des fonctionnalités

Ok Yannick, on sait déjà tout ça, où veux-tu en venir ?

Je veux en venir que j’ai l’impression que depuis la délocalisation des équipes de développements en Inde cet esprit s’est fortement perdu.

Je vais vous montrer un exemple qui m’avait littéralement assommé le jour où j’ai vu ça, sans trouver le temps avant aujourd’hui d’en parler.
Voici le code pour l’analyse statistique des factures :

def init(self, cr):
        tools.drop_view_if_exists(cr, 'account_invoice_report')
        cr.execute("""
            create or replace view account_invoice_report as (
                 select min(ail.id) as id,
                    ai.date_invoice as date,
                    to_char(ai.date_invoice, 'YYYY') as year,
                    to_char(ai.date_invoice, 'MM') as month,
                    to_char(ai.date_invoice, 'YYYY-MM-DD') as day,
                    ail.product_id,
                    ai.partner_id as partner_id,
                    ai.payment_term as payment_term,
                    ai.period_id as period_id,
                    (case when u.uom_type not in ('reference') then
                        (select name from product_uom where uom_type='reference' and active and category_id=u.category_id LIMIT 1)
                    else
                        u.name
                    end) as uom_name,
                    ai.currency_id as currency_id,
                    ai.journal_id as journal_id,
                    ai.fiscal_position as fiscal_position,
                    ai.user_id as user_id,
                    ai.company_id as company_id,
                    count(ail.*) as nbr,
                    ai.type as type,
                    ai.state,
                    pt.categ_id,
                    ai.date_due as date_due,
                    ai.address_contact_id as address_contact_id,
                    ai.address_invoice_id as address_invoice_id,
                    ai.account_id as account_id,
                    ai.partner_bank_id as partner_bank_id,
                    sum(case when ai.type in ('out_refund','in_invoice') then
                         ail.quantity / u.factor * -1
                        else
                         ail.quantity / u.factor
                        end) as product_qty,
                    sum(case when ai.type in ('out_refund','in_invoice') then
                         ail.quantity*ail.price_unit * -1
                        else
                         ail.quantity*ail.price_unit
                        end) / cr.rate as price_total,
                    sum(case when ai.type in ('out_refund','in_invoice') then
                         ai.amount_total * -1
                        else
                         ai.amount_total
                         end) / (CASE WHEN
                              (select count(l.id) from account_invoice_line as l
                               left join account_invoice as a ON (a.id=l.invoice_id)
                               where a.id=ai.id) <> 0
                            THEN
                              (select count(l.id) from account_invoice_line as l
                               left join account_invoice as a ON (a.id=l.invoice_id)
                               where a.id=ai.id)
                            ELSE 1
                            END) / cr.rate as price_total_tax,
                    (case when ai.type in ('out_refund','in_invoice') then
                      sum(ail.quantity*ail.price_unit*-1)
                    else
                      sum(ail.quantity*ail.price_unit)
                    end) / (CASE WHEN
                         (case when ai.type in ('out_refund','in_invoice')
                          then sum(ail.quantity/u.factor*-1)
                          else sum(ail.quantity/u.factor) end) <> 0
                       THEN
                         (case when ai.type in ('out_refund','in_invoice')
                          then sum(ail.quantity/u.factor*-1)
                          else sum(ail.quantity/u.factor) end)
                       ELSE 1
                       END)
                     / cr.rate as price_average,

                    cr.rate as currency_rate,
                    sum((select extract(epoch from avg(date_trunc('day',aml.date_created)-date_trunc('day',l.create_date)))/(24*60*60)::decimal(16,2)
                        from account_move_line as aml
                        left join account_invoice as a ON (a.move_id=aml.move_id)
                        left join account_invoice_line as l ON (a.id=l.invoice_id)
                        where a.id=ai.id)) as delay_to_pay,
                    sum((select extract(epoch from avg(date_trunc('day',a.date_due)-date_trunc('day',a.date_invoice)))/(24*60*60)::decimal(16,2)
                        from account_move_line as aml
                        left join account_invoice as a ON (a.move_id=aml.move_id)
                        left join account_invoice_line as l ON (a.id=l.invoice_id)
                        where a.id=ai.id)) as due_delay,
                    (case when ai.type in ('out_refund','in_invoice') then
                      ai.residual * -1
                    else
                      ai.residual
                    end)/ (CASE WHEN
                        (select count(l.id) from account_invoice_line as l
                         left join account_invoice as a ON (a.id=l.invoice_id)
                         where a.id=ai.id) <> 0
                       THEN
                        (select count(l.id) from account_invoice_line as l
                         left join account_invoice as a ON (a.id=l.invoice_id)
                         where a.id=ai.id)
                       ELSE 1
                       END) / cr.rate as residual
                from account_invoice_line as ail
                left join account_invoice as ai ON (ai.id=ail.invoice_id)
                left join product_template pt on (pt.id=ail.product_id)
                left join product_uom u on (u.id=ail.uos_id),
                res_currency_rate cr
                where cr.id in (select id from res_currency_rate cr2  where (cr2.currency_id = ai.currency_id)
                and ((ai.date_invoice is not null and cr.name <= ai.date_invoice) or (ai.date_invoice is null and cr.name <= NOW())) limit 1)
                group by ail.product_id,
                    ai.date_invoice,
                    ai.id,
                    cr.rate,
                    to_char(ai.date_invoice, 'YYYY'),
                    to_char(ai.date_invoice, 'MM'),
                    to_char(ai.date_invoice, 'YYYY-MM-DD'),
                    ai.partner_id,
                    ai.payment_term,
                    ai.period_id,
                    u.name,
                    ai.currency_id,
                    ai.journal_id,
                    ai.fiscal_position,
                    ai.user_id,
                    ai.company_id,
                    ai.type,
                    ai.state,
                    pt.categ_id,
                    ai.date_due,
                    ai.address_contact_id,
                    ai.address_invoice_id,
                    ai.account_id,
                    ai.partner_bank_id,
                    ai.residual,
                    ai.amount_total,
                    u.uom_type,
                    u.category_id
            )
        """)

134 lignes d’une pure requête SQL, juste pour créer une analyse statistique…
Alors ok c’est peut-être nécessaire, mais c’est dans le framework qu’on devrait retrouver ce code, pas dans l’un des modules fonctionnels ! Là on devrait juste lui dire voilà le nom de l’objet, voilà les champs à analyser et débrouille toi. Je ne reconnais pas du tout le code habituellement ultra-simple d’OpenERP dans ces 134 lignes, juste du code fait sans aucune réflexion préalable.

Quelle conséquence ? Moi, simple consultant fonctionnel, que ce passe-t il si je veux simplement rajouter un champ supplémentaire à analyser ? Ou créer une nouvelle analyse statistique sur un autre objet ? Ben je peux pas, obligé d’envoyer au développeurs qui vont me réécrire les 134lignes (ce qui pose un problème encore pire, j’y reviens).

Depuis quelques années, on constate qu’il n’y a plus de gros efforts au niveau du framework. Alors ok je soutiens à 300% le client web, la paye et encore plus le POS. Mais je commence à entrevoir de gros problèmes à l’avenir pour OpenERP qui risque de s’effondrer sur ses fondations, c’est-à-dire le framework.

Soyons clair, je ne suis pas en train de dire que OpenERP est mal codé, qu’il est pas « pythonic » ou autre. Je n’ai pas les compétences en développement pour ça et d’autres s’en chargent (dédicace aux gens de Tryton qui j’en suis sûr doivent se régaler avec ce billet…).
Ce que je veux dire c’est que les fonctionnels devraient pouvoir modifier encore beaucoup plus de fonctionnalités sur OpenERP que maintenant.

Ce qui se passe quand on passe une commande d’un statut à un autre, comme envoyer un mail, rajouter un module possible dans un wizard de configuration, ou encore transférer la valeur du champ qu’on vient de créer dans sale.order à son account.invoice etc…
Il y a encore plein de choses que nous pouvons faire, faire gérer  par OpenObject  encore plus de fonctions récurrentes dans les ERPs jusqu’à finir par limiter la part de code Python au strict minimum.

Qui plus est, cela sera je pense un bon moyen de limiter la quantité de code qui est pondu chaque jour par les équipes indiennes. Je suis désolé de dire ça mais à pas mal d’endroits j’ai parfois l’impression, pardonnez-moi l’expression, qu’ils ont juste « pissé du code » comme si ils étaient payé à la ligne. Je crains fortement que cela ne fasse courir un risque et qu’à terme on n’arrive plus à maintenir le logiciel.
Cadrer les équipes de développeurs dans OpenObject pour les développements les plus courants fera qu’ils n’auront qu’une manière de développer la fonctionnalité et qui sera la manière la plus simple. Cela permettra par la suite de pouvoir bien plus facilement maintenir le logiciel ou surtout refondre en profondeur un module (et cela arrivera pour certains, on peut en être sûr, on sera de plus en plus exigeant avec OpenERP au fur et à mesure que sa popularité grandit).

Et le pire c’est que pour certaines fonctionnalités elles sont déjà gérées par OpenObject. Par exemple la création d’une nouvelle facture avec les valeurs d’un bon de commande via ir.actions.server; mais comme dans les modules certifiés on ne l’utilise pas, personne n’utilise ces fonctionnalités…

Bref, je vais être clair sur ce que je propose : Dans un premier temps, il faut réécrire tous les modules certifiés pour qu’ils utilisent ir.actions.server ou utiliser un système équivalent. Ensuite continuer à améliorer le framework jusqu’à ce que le code Python dans les modules soit réduit au strict minimum.

Je suis certain que la majorité des lecteurs vont s’insurger contre cette idée, trop de travail, il vaut mieux laisser certaines fonctions en python pour avoir plus de flexibilité etc…  Pas de soucis, j’essaie juste de lancer le débat et j’espère juste qu’il va aboutir à des conclusions intéressantes.

J’aimerais néanmoins pointer du doigt un dernier problème, le pire je pense, et qui peut justement être résolu par une extension des possibilités d’OpenObject.

Prenons un partenaire A, très connu et top contributeur. Il a développé un module qui est très utilisé par d’autres membres de la communauté.

Dans son module, il a créé un champ X dans sale.order et dans account.invoice, et il a fait en sorte que sa valeur dans sale.order soit transmise à son account.invoice.

Voici le code qu’il a dû faire :

    def _make_invoice(self, cr, uid, order, lines, context=None):
        journal_obj = self.pool.get('account.journal')
        inv_obj = self.pool.get('account.invoice')
        obj_invoice_line = self.pool.get('account.invoice.line')
        if context is None:
            context = {}

        journal_ids = journal_obj.search(cr, uid, [('type', '=', 'sale'), ('company_id', '=', order.company_id.id)], limit=1)
        if not journal_ids:
            raise osv.except_osv(_('Error !'),
                _('There is no sales journal defined for this company: "%s" (id:%d)') % (order.company_id.name, order.company_id.id))
        a = order.partner_id.property_account_receivable.id
        pay_term = order.payment_term and order.payment_term.id or False
        invoiced_sale_line_ids = self.pool.get('sale.order.line').search(cr, uid, [('order_id', '=', order.id), ('invoiced', '=', True)], context=context)
        from_line_invoice_ids = []
        for invoiced_sale_line_id in self.pool.get('sale.order.line').browse(cr, uid, invoiced_sale_line_ids, context=context):
            for invoice_line_id in invoiced_sale_line_id.invoice_lines:
                if invoice_line_id.invoice_id.id not in from_line_invoice_ids:
                    from_line_invoice_ids.append(invoice_line_id.invoice_id.id)
        for preinv in order.invoice_ids:
            if preinv.state not in ('cancel',) and preinv.id not in from_line_invoice_ids:
                for preline in preinv.invoice_line:
                    inv_line_id = obj_invoice_line.copy(cr, uid, preline.id, {'invoice_id': False, 'price_unit': -preline.price_unit})
                    lines.append(inv_line_id)
        inv = {
            'name': order.client_order_ref or '',
            'origin': order.name,
            'type': 'out_invoice',
            'reference': order.client_order_ref or order.name,
            'account_id': a,
            'partner_id': order.partner_id.id,
            'journal_id': journal_ids[0],
            'address_invoice_id': order.partner_invoice_id.id,
            'address_contact_id': order.partner_order_id.id,
            'invoice_line': [(6, 0, lines)],
            'currency_id': order.pricelist_id.currency_id.id,
            'comment': order.note,
            'payment_term': pay_term,
            'fiscal_position': order.fiscal_position.id or order.partner_id.property_account_position.id,
            'date_invoice': context.get('date_invoice',False),
            'company_id': order.company_id.id,
            'user_id': order.user_id and order.user_id.id or False,
            'champ_x': order.champ_x
        }
        inv.update(self._inv_get(cr, uid, order))
        inv_id = inv_obj.create(cr, uid, inv, context=context)
        data = inv_obj.onchange_payment_term_date_invoice(cr, uid, [inv_id], pay_term, time.strftime('%Y-%m-%d'))
        if data.get('value', False):
            inv_obj.write(cr, uid, [inv_id], data['value'], context=context)
        inv_obj.button_compute(cr, uid, [inv_id])
        return inv_id

50 lignes, alors qu’en fait la seule ligne qu’il a vraiment codé c’est la ligne en rouge ici. Et encore on a eu de la chance, la fonction d’origine aurait pu être bien plus longue.
Comme il n’est pas possible via son module d’insérer son code directement, il a dû copier-coller l’ensemble de la fonction du module sale pour rajouter sa petite ligne.

Et là le vrai problème arrive. Pas de chance les devs de l’éditeur modifient par la suite la fonction d’origine et d’autres fonctions ailleurs, de sorte que l’ancienne provoque désormais un bug. Notre partenaire A doit désormais refaire le copier-coller de la fonction d’origine s’il ne veut pas que son module soit considéré comme buggé.

Cela oblige notre partenaire à faire de la veille permanente sur tous ses modules, juste pour vérifier qu’OpenERP SA n’a pas modifié la fonction d’origine. Et vu que les fonctions sont de plus en plus longues, les risques de bugs le sont d’autant plus. Enfin, cas ultime, imaginez si un partenaire B créé un module qui hérite de la fonction du module de A… Ce que je veux dire par là, c’est que c’est tout l’écosystème des modules communautaire qui est instable à cause de cette surutilisation des fonctions Python qui ne sont pas DU TOUT adaptées à un système aussi modulaire qu’OpenERP.

Je pense qu’on est en plein dedans aujourd’hui, on constate que de plus en plus de modules de qualité arrivent sur OpenERP, mais ils sont rapidement buggés dès que OpenERP SA fait des changements dans les fonctions des modules certifiés, qui deviennent eux-mêmes de plus en plus long et complexe.

Il faut à tout prix trouver une autre solution pour les développeurs de module pour ne pas avoir à recopier la fonction d’origine quand ils veulent juste rajouter quelques détails en plus comme un champ à transférer. Et pour moi la seule solution viable, comme il est clair qu’on ne pourra pas faire ça directement dans le code Python, c’est donc de diminuer la part de ce code Python en étendant et surtout utilisant les possibilités d’OpenObject qui peuvent je pense être encore améliorés.

Merci d’avoir tenu cette lecture. J’exprime ici ma conviction personnelle, qui est un peu isolée. Je ne suis pas développeur, aussi si j’ai peut-être dit de grosses conneries, n’hésitez pas à me l’indiquer dans les commentaires.

    def init(self, cr):
79
        tools.drop_view_if_exists(cr, 'account_invoice_report')
80
        cr.execute("""
81
            create or replace view account_invoice_report as (
82
                 select min(ail.id) as id,
83
                    ai.date_invoice as date,
84
                    to_char(ai.date_invoice, 'YYYY') as year,
85
                    to_char(ai.date_invoice, 'MM') as month,
86
                    to_char(ai.date_invoice, 'YYYY-MM-DD') as day,
87
                    ail.product_id,
88
                    ai.partner_id as partner_id,
89
                    ai.payment_term as payment_term,
90
                    ai.period_id as period_id,
91
                    (case when u.uom_type not in ('reference') then
92
                        (select name from product_uom where uom_type='reference' and active and category_id=u.category_id LIMIT 1)
93
                    else
94
                        u.name
95
                    end) as uom_name,
96
                    ai.currency_id as currency_id,
97
                    ai.journal_id as journal_id,
98
                    ai.fiscal_position as fiscal_position,
99
                    ai.user_id as user_id,
100
                    ai.company_id as company_id,
101
                    count(ail.*) as nbr,
102
                    ai.type as type,
103
                    ai.state,
104
                    pt.categ_id,
105
                    ai.date_due as date_due,
106
                    ai.address_contact_id as address_contact_id,
107
                    ai.address_invoice_id as address_invoice_id,
108
                    ai.account_id as account_id,
109
                    ail.account_id as account_line_id,
110
                    ai.partner_bank_id as partner_bank_id,
111
                    sum(case when ai.type in ('out_refund','in_invoice') then
112
                         -ail.quantity / u.factor
113
                        else
114
                         ail.quantity / u.factor
115
                        end) as product_qty,
116
117
                    sum(case when ai.type in ('out_refund','in_invoice') then
118
                         -ail.price_subtotal
119
                        else
120
                          ail.price_subtotal
121
                        end) / cr.rate as price_total,
122
123
                    (case when ai.type in ('out_refund','in_invoice') then
124
                      sum(-ail.price_subtotal)
125
                    else
126
                      sum(ail.price_subtotal)
127
                    end) / (CASE WHEN sum(ail.quantity/u.factor) <> 0
128
                       THEN
129
                         (case when ai.type in ('out_refund','in_invoice')
130
                          then sum(-ail.quantity/u.factor)
131
                          else sum(ail.quantity/u.factor) end)
132
                       ELSE 1
133
                       END)
134
                     / cr.rate as price_average,
135
136
                    cr.rate as currency_rate,
137
                    sum((select extract(epoch from avg(date_trunc('day',aml.date_created)-date_trunc('day',l.create_date)))/(24*60*60)::decimal(16,2)
138
                        from account_move_line as aml
139
                        left join account_invoice as a ON (a.move_id=aml.move_id)
140
                        left join account_invoice_line as l ON (a.id=l.invoice_id)
141
                        where a.id=ai.id)) as delay_to_pay,
142
                    sum((select extract(epoch from avg(date_trunc('day',a.date_due)-date_trunc('day',a.date_invoice)))/(24*60*60)::decimal(16,2)
143
                        from account_move_line as aml
144
                        left join account_invoice as a ON (a.move_id=aml.move_id)
145
                        left join account_invoice_line as l ON (a.id=l.invoice_id)
146
                        where a.id=ai.id)) as due_delay,
147
                    (case when ai.type in ('out_refund','in_invoice') then
148
                      -ai.residual
149
                    else
150
                      ai.residual
151
                    end)/ (CASE WHEN
152
                        (select count(l.id) from account_invoice_line as l
153
                         left join account_invoice as a ON (a.id=l.invoice_id)
154
                         where a.id=ai.id) <> 0
155
                       THEN
156
                        (select count(l.id) from account_invoice_line as l
157
                         left join account_invoice as a ON (a.id=l.invoice_id)
158
                         where a.id=ai.id)
159
                       ELSE 1
160
                       END) / cr.rate as residual
161
                from account_invoice_line as ail
162
                left join account_invoice as ai ON (ai.id=ail.invoice_id)
163
                left join product_product pr on (pr.id=ail.product_id)
164
                left join product_template pt on (pt.id=pr.product_tmpl_id)
165
                left join product_uom u on (u.id=ail.uos_id),
166
                res_currency_rate cr
167
                where cr.id in (select id from res_currency_rate cr2  where (cr2.currency_id = ai.currency_id)
168
                and ((ai.date_invoice is not null and cr.name <= ai.date_invoice) or (ai.date_invoice is null and cr.name <= NOW())) limit 1)
169
                group by ail.product_id,
170
                    ai.date_invoice,
171
                    ai.id,
172
                    cr.rate,
173
                    to_char(ai.date_invoice, 'YYYY'),
174
                    to_char(ai.date_invoice, 'MM'),
175
                    to_char(ai.date_invoice, 'YYYY-MM-DD'),
176
                    ai.partner_id,
177
                    ai.payment_term,
178
                    ai.period_id,
179
                    u.name,
180
                    ai.currency_id,
181
                    ai.journal_id,
182
                    ai.fiscal_position,
183
                    ai.user_id,
184
                    ai.company_id,
185
                    ai.type,
186
                    ai.state,
187
                    pt.categ_id,
188
                    ai.date_due,
189
                    ai.address_contact_id,
190
                    ai.address_invoice_id,
191
                    ai.account_id,
192
                    ail.account_id,
193
                    ai.partner_bank_id,
194
                    ai.residual,
195
                    ai.amount_total,
196
                    u.uom_type,
197
                    u.category_id
198
            )
199
        """)

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>